From 9ab04188cc220d21ac38207a8d4d1422592babac Mon Sep 17 00:00:00 2001 From: Manfred Karrer Date: Wed, 15 Jun 2016 22:05:23 +0200 Subject: [PATCH] Add check at OfferAvailability check for arbitrator and for price tolerance --- .../TradePriceOutOfToleranceException.java | 7 ++++ .../java/io/bitsquare/trade/offer/Offer.java | 20 ++++++++++ .../trade/offer/OpenOfferManager.java | 29 ++++++++++++++- .../availability/AvailabilityResult.java | 9 +++++ .../availability/OfferAvailabilityModel.java | 12 ++++-- .../messages/OfferAvailabilityRequest.java | 4 +- .../messages/OfferAvailabilityResponse.java | 9 +++-- .../ProcessOfferAvailabilityResponse.java | 9 +++-- .../tasks/SendOfferAvailabilityRequest.java | 2 +- .../offerer/ProcessPayDepositRequest.java | 37 +++++++------------ .../main/offer/takeoffer/TakeOfferView.java | 7 ++-- 11 files changed, 104 insertions(+), 41 deletions(-) create mode 100644 core/src/main/java/io/bitsquare/trade/exceptions/TradePriceOutOfToleranceException.java create mode 100644 core/src/main/java/io/bitsquare/trade/protocol/availability/AvailabilityResult.java diff --git a/core/src/main/java/io/bitsquare/trade/exceptions/TradePriceOutOfToleranceException.java b/core/src/main/java/io/bitsquare/trade/exceptions/TradePriceOutOfToleranceException.java new file mode 100644 index 0000000000..d9f82abdd2 --- /dev/null +++ b/core/src/main/java/io/bitsquare/trade/exceptions/TradePriceOutOfToleranceException.java @@ -0,0 +1,7 @@ +package io.bitsquare.trade.exceptions; + +public class TradePriceOutOfToleranceException extends Exception { + public TradePriceOutOfToleranceException(String message) { + super(message); + } +} diff --git a/core/src/main/java/io/bitsquare/trade/offer/Offer.java b/core/src/main/java/io/bitsquare/trade/offer/Offer.java index d63c07ca57..096a49bbb7 100644 --- a/core/src/main/java/io/bitsquare/trade/offer/Offer.java +++ b/core/src/main/java/io/bitsquare/trade/offer/Offer.java @@ -30,6 +30,7 @@ import io.bitsquare.p2p.NodeAddress; import io.bitsquare.p2p.storage.payload.RequiresOwnerIsOnlinePayload; import io.bitsquare.p2p.storage.payload.StoragePayload; import io.bitsquare.payment.PaymentMethod; +import io.bitsquare.trade.exceptions.TradePriceOutOfToleranceException; import io.bitsquare.trade.protocol.availability.OfferAvailabilityModel; import io.bitsquare.trade.protocol.availability.OfferAvailabilityProtocol; import javafx.beans.property.*; @@ -381,6 +382,25 @@ public final class Offer implements StoragePayload, RequiresOwnerIsOnlinePayload } } + public void checkTradePriceTolerance(long takersTradePrice) throws TradePriceOutOfToleranceException, IllegalArgumentException { + checkArgument(takersTradePrice > 0, "takersTradePrice must be positive"); + Fiat tradePriceAsFiat = Fiat.valueOf(getCurrencyCode(), takersTradePrice); + Fiat offerPriceAsFiat = getPrice(); + checkArgument(offerPriceAsFiat != null, "offerPriceAsFiat must not be null"); + double factor = (double) takersTradePrice / (double) offerPriceAsFiat.value; + // We allow max. 2 % difference between own offer price calculation and takers calculation. + // Market price might be different at offerers and takers side so we need a bit of tolerance. + // The tolerance will get smaller once we have multiple price feeds avoiding fast price fluctuations + // from one provider. + if (Math.abs(1 - factor) > 0.02) { + String msg = "Taker's trade price is too far away from our calculated price based on the market price.\n" + + "tradePriceAsFiat=" + tradePriceAsFiat.toFriendlyString() + "\n" + + "offerPriceAsFiat=" + offerPriceAsFiat.toFriendlyString(); + log.warn(msg); + throw new TradePriceOutOfToleranceException(msg); + } + } + public double getMarketPriceMargin() { return marketPriceMargin; } diff --git a/core/src/main/java/io/bitsquare/trade/offer/OpenOfferManager.java b/core/src/main/java/io/bitsquare/trade/offer/OpenOfferManager.java index 8f2a2c9da0..ff356c25b0 100644 --- a/core/src/main/java/io/bitsquare/trade/offer/OpenOfferManager.java +++ b/core/src/main/java/io/bitsquare/trade/offer/OpenOfferManager.java @@ -40,7 +40,9 @@ import io.bitsquare.p2p.peers.PeerManager; import io.bitsquare.storage.Storage; import io.bitsquare.trade.TradableList; import io.bitsquare.trade.closed.ClosedTradableManager; +import io.bitsquare.trade.exceptions.TradePriceOutOfToleranceException; import io.bitsquare.trade.handlers.TransactionResultHandler; +import io.bitsquare.trade.protocol.availability.AvailabilityResult; import io.bitsquare.trade.protocol.availability.messages.OfferAvailabilityRequest; import io.bitsquare.trade.protocol.availability.messages.OfferAvailabilityResponse; import io.bitsquare.trade.protocol.placeoffer.PlaceOfferModel; @@ -348,11 +350,34 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } Optional openOfferOptional = findOpenOffer(message.offerId); - boolean isAvailable = openOfferOptional.isPresent() && openOfferOptional.get().getState() == OpenOffer.State.AVAILABLE; + AvailabilityResult availabilityResult; + if (openOfferOptional.isPresent() && openOfferOptional.get().getState() == OpenOffer.State.AVAILABLE) { + availabilityResult = AvailabilityResult.AVAILABLE; + List acceptedArbitrators = user.getAcceptedArbitratorAddresses(); + if (acceptedArbitrators != null && !acceptedArbitrators.isEmpty()) { + // Check also tradePrice to avoid failures after taker fee is paid caused by a too big difference + // in trade price between the peers. Also here poor connectivity might cause market price API connection + // losses and therefore an outdated market price. + try { + openOfferOptional.get().getOffer().checkTradePriceTolerance(message.takersTradePrice); + } catch (TradePriceOutOfToleranceException e) { + log.warn("Trade price check failed because takers price is outside out tolerance."); + availabilityResult = AvailabilityResult.PRICE_OUT_OF_TOLERANCE; + } catch (Throwable e) { + log.warn("Trade price check failed. " + e.getMessage()); + availabilityResult = AvailabilityResult.UNKNOWN_FAILURE; + } + } else { + log.warn("acceptedArbitrators is null or empty: acceptedArbitrators=" + acceptedArbitrators); + availabilityResult = AvailabilityResult.NO_ARBITRATORS; + } + } else { + availabilityResult = AvailabilityResult.OFFER_TAKEN; + } try { p2PService.sendEncryptedDirectMessage(sender, message.getPubKeyRing(), - new OfferAvailabilityResponse(message.offerId, isAvailable), + new OfferAvailabilityResponse(message.offerId, availabilityResult), new SendDirectMessageListener() { @Override public void onArrived() { diff --git a/core/src/main/java/io/bitsquare/trade/protocol/availability/AvailabilityResult.java b/core/src/main/java/io/bitsquare/trade/protocol/availability/AvailabilityResult.java new file mode 100644 index 0000000000..270ed10439 --- /dev/null +++ b/core/src/main/java/io/bitsquare/trade/protocol/availability/AvailabilityResult.java @@ -0,0 +1,9 @@ +package io.bitsquare.trade.protocol.availability; + +public enum AvailabilityResult { + AVAILABLE, + OFFER_TAKEN, + PRICE_OUT_OF_TOLERANCE, + NO_ARBITRATORS, + UNKNOWN_FAILURE +} diff --git a/core/src/main/java/io/bitsquare/trade/protocol/availability/OfferAvailabilityModel.java b/core/src/main/java/io/bitsquare/trade/protocol/availability/OfferAvailabilityModel.java index 0418d639be..f4d77f6680 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/availability/OfferAvailabilityModel.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/availability/OfferAvailabilityModel.java @@ -22,7 +22,7 @@ import io.bitsquare.common.taskrunner.Model; import io.bitsquare.p2p.NodeAddress; import io.bitsquare.p2p.P2PService; import io.bitsquare.trade.offer.Offer; -import io.bitsquare.trade.protocol.availability.messages.OfferMessage; +import io.bitsquare.trade.protocol.availability.messages.OfferAvailabilityResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,7 +34,7 @@ public class OfferAvailabilityModel implements Model { public final P2PService p2PService; private NodeAddress peerNodeAddress; - private OfferMessage message; + private OfferAvailabilityResponse message; public OfferAvailabilityModel(Offer offer, PubKeyRing pubKeyRing, @@ -52,14 +52,18 @@ public class OfferAvailabilityModel implements Model { this.peerNodeAddress = peerNodeAddress; } - public void setMessage(OfferMessage message) { + public void setMessage(OfferAvailabilityResponse message) { this.message = message; } - public OfferMessage getMessage() { + public OfferAvailabilityResponse getMessage() { return message; } + public long getTakersTradePrice() { + return offer.getPrice() != null ? offer.getPrice().value : 0; + } + @Override public void persist() { } diff --git a/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityRequest.java b/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityRequest.java index a1d65b57b2..ccb784760d 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityRequest.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityRequest.java @@ -25,10 +25,12 @@ public final class OfferAvailabilityRequest extends OfferMessage { private static final long serialVersionUID = Version.P2P_NETWORK_VERSION; private final PubKeyRing pubKeyRing; + public final long takersTradePrice; - public OfferAvailabilityRequest(String offerId, PubKeyRing pubKeyRing) { + public OfferAvailabilityRequest(String offerId, PubKeyRing pubKeyRing, long takersTradePrice) { super(offerId); this.pubKeyRing = pubKeyRing; + this.takersTradePrice = takersTradePrice; } public PubKeyRing getPubKeyRing() { diff --git a/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityResponse.java b/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityResponse.java index 1f7baff2a6..504b3459bb 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityResponse.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/availability/messages/OfferAvailabilityResponse.java @@ -18,22 +18,23 @@ package io.bitsquare.trade.protocol.availability.messages; import io.bitsquare.app.Version; +import io.bitsquare.trade.protocol.availability.AvailabilityResult; public final class OfferAvailabilityResponse extends OfferMessage { // That object is sent over the wire, so we need to take care of version compatibility. private static final long serialVersionUID = Version.P2P_NETWORK_VERSION; - public final boolean isAvailable; + public final AvailabilityResult availabilityResult; - public OfferAvailabilityResponse(String offerId, boolean isAvailable) { + public OfferAvailabilityResponse(String offerId, AvailabilityResult availabilityResult) { super(offerId); - this.isAvailable = isAvailable; + this.availabilityResult = availabilityResult; } @Override public String toString() { return "OfferAvailabilityResponse{" + - "isAvailable=" + isAvailable + + "availabilityResult=" + availabilityResult + "} " + super.toString(); } } \ No newline at end of file diff --git a/core/src/main/java/io/bitsquare/trade/protocol/availability/tasks/ProcessOfferAvailabilityResponse.java b/core/src/main/java/io/bitsquare/trade/protocol/availability/tasks/ProcessOfferAvailabilityResponse.java index 073317a80a..efaecf8758 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/availability/tasks/ProcessOfferAvailabilityResponse.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/availability/tasks/ProcessOfferAvailabilityResponse.java @@ -20,6 +20,7 @@ package io.bitsquare.trade.protocol.availability.tasks; import io.bitsquare.common.taskrunner.Task; import io.bitsquare.common.taskrunner.TaskRunner; import io.bitsquare.trade.offer.Offer; +import io.bitsquare.trade.protocol.availability.AvailabilityResult; import io.bitsquare.trade.protocol.availability.OfferAvailabilityModel; import io.bitsquare.trade.protocol.availability.messages.OfferAvailabilityResponse; import org.slf4j.Logger; @@ -36,13 +37,15 @@ public class ProcessOfferAvailabilityResponse extends Task { model.p2PService.sendEncryptedDirectMessage(model.getPeerNodeAddress(), model.offer.getPubKeyRing(), - new OfferAvailabilityRequest(model.offer.getId(), model.pubKeyRing), + new OfferAvailabilityRequest(model.offer.getId(), model.pubKeyRing, model.getTakersTradePrice()), new SendDirectMessageListener() { @Override public void onArrived() { diff --git a/core/src/main/java/io/bitsquare/trade/protocol/trade/tasks/offerer/ProcessPayDepositRequest.java b/core/src/main/java/io/bitsquare/trade/protocol/trade/tasks/offerer/ProcessPayDepositRequest.java index 359361f022..4f9797050b 100644 --- a/core/src/main/java/io/bitsquare/trade/protocol/trade/tasks/offerer/ProcessPayDepositRequest.java +++ b/core/src/main/java/io/bitsquare/trade/protocol/trade/tasks/offerer/ProcessPayDepositRequest.java @@ -20,10 +20,10 @@ package io.bitsquare.trade.protocol.trade.tasks.offerer; import io.bitsquare.common.taskrunner.TaskRunner; import io.bitsquare.payment.PaymentAccountContractData; import io.bitsquare.trade.Trade; +import io.bitsquare.trade.exceptions.TradePriceOutOfToleranceException; import io.bitsquare.trade.protocol.trade.messages.PayDepositRequest; import io.bitsquare.trade.protocol.trade.tasks.TradeTask; import org.bitcoinj.core.Coin; -import org.bitcoinj.utils.Fiat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,8 +45,8 @@ public class ProcessPayDepositRequest extends TradeTask { runInterceptHook(); log.debug("current trade state " + trade.getState()); PayDepositRequest payDepositRequest = (PayDepositRequest) processModel.getTradeMessage(); - checkTradeId(processModel.getId(), payDepositRequest); checkNotNull(payDepositRequest); + checkTradeId(processModel.getId(), payDepositRequest); processModel.tradingPeer.setRawTransactionInputs(checkNotNull(payDepositRequest.rawTransactionInputs)); checkArgument(payDepositRequest.rawTransactionInputs.size() > 0); @@ -65,34 +65,25 @@ public class ProcessPayDepositRequest extends TradeTask { processModel.tradingPeer.setAccountId(nonEmptyStringOf(payDepositRequest.takerAccountId)); trade.setTakeOfferFeeTxId(nonEmptyStringOf(payDepositRequest.takeOfferFeeTxId)); processModel.setTakerAcceptedArbitratorNodeAddresses(checkNotNull(payDepositRequest.acceptedArbitratorNodeAddresses)); - if (payDepositRequest.acceptedArbitratorNodeAddresses.size() < 1) - failed("acceptedArbitratorNames size must be at least 1"); + if (payDepositRequest.acceptedArbitratorNodeAddresses.isEmpty()) + failed("acceptedArbitratorNames must not be empty"); trade.setArbitratorNodeAddress(checkNotNull(payDepositRequest.arbitratorNodeAddress)); - long takersTradePrice = payDepositRequest.tradePrice; - checkArgument(takersTradePrice > 0); - Fiat tradePriceAsFiat = Fiat.valueOf(trade.getOffer().getCurrencyCode(), takersTradePrice); - Fiat offerPriceAsFiat = trade.getOffer().getPrice(); - checkArgument(offerPriceAsFiat != null, "offerPriceAsFiat must not be null"); - double factor = (double) takersTradePrice / (double) offerPriceAsFiat.value; - // We allow max. 2 % difference between own offer price calculation and takers calculation. - // Market price might be different at offerers and takers side so we need a bit of tolerance. - // The tolerance will get smaller once we have multiple price feeds avoiding fast price fluctuations - // from one provider. - if (Math.abs(1 - factor) > 0.02) { - String msg = "Takers tradePrice is outside our market price tolerance.\n" + - "tradePriceAsFiat=" + tradePriceAsFiat.toFriendlyString() + "\n" + - "offerPriceAsFiat=" + offerPriceAsFiat.toFriendlyString(); - log.warn(msg); - failed(msg); + try { + long takersTradePrice = payDepositRequest.tradePrice; + trade.getOffer().checkTradePriceTolerance(takersTradePrice); + trade.setTradePrice(takersTradePrice); + } catch (TradePriceOutOfToleranceException e) { + failed(e.getMessage()); + } catch (Throwable e2) { + failed(e2); } - trade.setTradePrice(takersTradePrice); - checkArgument(payDepositRequest.tradeAmount > 0); trade.setTradeAmount(Coin.valueOf(payDepositRequest.tradeAmount)); - // update to the latest peer address of our peer if the payDepositRequest is correct + // check and update to the latest peer address of our peer if the payDepositRequest is correct + checkArgument(payDepositRequest.getSenderNodeAddress().equals(processModel.getTempTradingPeerNodeAddress())); trade.setTradingPeerNodeAddress(processModel.getTempTradingPeerNodeAddress()); removeMailboxMessageAfterProcessing(); diff --git a/gui/src/main/java/io/bitsquare/gui/main/offer/takeoffer/TakeOfferView.java b/gui/src/main/java/io/bitsquare/gui/main/offer/takeoffer/TakeOfferView.java index 2fcb16b516..5dcaeb7be7 100644 --- a/gui/src/main/java/io/bitsquare/gui/main/offer/takeoffer/TakeOfferView.java +++ b/gui/src/main/java/io/bitsquare/gui/main/offer/takeoffer/TakeOfferView.java @@ -278,17 +278,18 @@ public class TakeOfferView extends ActivatableViewAndModel model.onTakeOffer(() -> { offerDetailsWindow.hide(); offerDetailsWindowDisplayed = false; }) ).show(model.getOffer(), model.dataModel.amountAsCoin.get(), model.dataModel.tradePrice); - else + offerDetailsWindowDisplayed = true; + } else { model.onTakeOffer(() -> { }); - offerDetailsWindowDisplayed = true; + } } else { new Popup().warning("You have no arbitrator selected.\n" + "You need to select at least one arbitrator.")