Skip to content

Commit

Permalink
Refactor offer/trade related classes in core and desktop
Browse files Browse the repository at this point in the history 
These refactoring changes are for reducing existing and potential
duplication coming with the addition of new trading protocol support
in the gRPC API.  Some minor styling and logic simplification changes
are also include.

- Convert OfferUtil to injected singleton, and move various offer related
  utility methods into it.

- Delete both MakerFeeProvider classes, which were wrappers around the same
  static old OfferUtil method.

- Inject OfferUtil into CreateOfferDataModel, CreateOfferViewModel,
  TakeOfferDataModel, TakeOfferViewModel, MutableOfferDataModel,
  MutableOfferViewModel, OfferDataModel, EditOfferDataModel,
  EditOfferViewModel

- Refactor TakeOfferViewModel

	Use OfferUtil, remove unused fields & methods.
	Made minor logic simplification, style and formatting changes.

- MutableOfferDataModel

	Made minor logic simplification, style and formatting changes.

- MutableOfferView uses new paymentAccount.isHalCashAccount().

- MutableOfferViewModel

	Refactored to use new VolumeUtil, CoinUtil, OfferUtil.
	Removed unused fields & accessors.
	Made minor style change.

- Refactored OfferDataModel to use new OfferUtil

- Refactor CreateOfferService

	Inject and use OfferUtil
	Move some utility methods to OfferUtil
	Remove unused fields

- Offer

	Refactored to use new VolumeUtil for volume calculations.
	Made stateProperty and errorMessageProperty fields private.

- PaymentAccount

	Moved isHalCashAccount type check to this class.
	Moved getTradeCurrency logic to this class.

- Contract, radeStatistics2, TradeStatistics3

	Refactored to use new VolumeUtil for volume calculations.

- Trade

	Refactored to use new VolumeUtil for volume calculations.
	Made minor logic simplification, style and formatting changes.

- CoinUtil

	Moved some coin utility methods into this class

- CoinUtilTest

	Moved (coin related) tests from CoinCryptoUtilsTest and OfferUtilTest
	into CoinUtilTest, and deleted OfferUtilTest, CoinCryptoUtilsTest.

- Adjust create and edit offer tests to model refactoring
  • Loading branch information
@ghubstan
ghubstan committed Oct 20, 2020
1 parent 50e2c89 commit ab6be23
Show file tree
Hide file tree
Showing 27 changed files with 656 additions and 645 deletions.
84 changes: 19 additions & 65 deletions core/src/main/java/bisq/core/offer/CreateOfferService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,16 @@

package bisq.core.offer;

import bisq.core.account.witness.AccountAgeWitnessService;
import bisq.core.btc.TxFeeEstimationService;
import bisq.core.btc.wallet.BsqWalletService;
import bisq.core.btc.wallet.BtcWalletService;
import bisq.core.btc.wallet.Restrictions;
import bisq.core.filter.FilterManager;
import bisq.core.locale.CurrencyUtil;
import bisq.core.locale.Res;
import bisq.core.monetary.Price;
import bisq.core.payment.HalCashAccount;
import bisq.core.payment.PaymentAccount;
import bisq.core.payment.PaymentAccountUtil;
import bisq.core.provider.price.MarketPrice;
import bisq.core.provider.price.PriceFeedService;
import bisq.core.trade.statistics.ReferralIdService;
import bisq.core.user.Preferences;
import bisq.core.user.User;
import bisq.core.util.coin.CoinUtil;
Expand Down Expand Up @@ -62,14 +57,9 @@
@Slf4j
@Singleton
public class CreateOfferService {
private final OfferUtil offerUtil;
private final TxFeeEstimationService txFeeEstimationService;
private final MakerFeeProvider makerFeeProvider;
private final BsqWalletService bsqWalletService;
private final Preferences preferences;
private final PriceFeedService priceFeedService;
private final AccountAgeWitnessService accountAgeWitnessService;
private final ReferralIdService referralIdService;
private final FilterManager filterManager;
private final P2PService p2PService;
private final PubKeyRing pubKeyRing;
private final User user;
Expand All @@ -81,26 +71,16 @@ public class CreateOfferService {
///////////////////////////////////////////////////////////////////////////////////////////

@Inject
public CreateOfferService(TxFeeEstimationService txFeeEstimationService,
MakerFeeProvider makerFeeProvider,
BsqWalletService bsqWalletService,
Preferences preferences,
public CreateOfferService(OfferUtil offerUtil,
TxFeeEstimationService txFeeEstimationService,
PriceFeedService priceFeedService,
AccountAgeWitnessService accountAgeWitnessService,
ReferralIdService referralIdService,
FilterManager filterManager,
P2PService p2PService,
PubKeyRing pubKeyRing,
User user,
BtcWalletService btcWalletService) {
this.offerUtil = offerUtil;
this.txFeeEstimationService = txFeeEstimationService;
this.makerFeeProvider = makerFeeProvider;
this.bsqWalletService = bsqWalletService;
this.preferences = preferences;
this.priceFeedService = priceFeedService;
this.accountAgeWitnessService = accountAgeWitnessService;
this.referralIdService = referralIdService;
this.filterManager = filterManager;
this.p2PService = p2PService;
this.pubKeyRing = pubKeyRing;
this.user = user;
Expand Down Expand Up @@ -161,7 +141,7 @@ public Offer createAndGetOffer(String offerId,
NodeAddress makerAddress = p2PService.getAddress();
boolean useMarketBasedPriceValue = useMarketBasedPrice &&
isMarketPriceAvailable(currencyCode) &&
!isHalCashAccount(paymentAccount);
!paymentAccount.isHalCashAccount();

long priceAsLong = price != null && !useMarketBasedPriceValue ? price.getValue() : 0L;
double marketPriceMarginParam = useMarketBasedPriceValue ? marketPriceMargin : 0;
Expand All @@ -185,11 +165,11 @@ public Offer createAndGetOffer(String offerId,
double sellerSecurityDeposit = getSellerSecurityDepositAsDouble(buyerSecurityDepositAsDouble);
Coin txFeeFromFeeService = getEstimatedFeeAndTxSize(amount, direction, buyerSecurityDepositAsDouble, sellerSecurityDeposit).first;
Coin txFeeToUse = txFee.isPositive() ? txFee : txFeeFromFeeService;
Coin makerFeeAsCoin = getMakerFee(amount);
boolean isCurrencyForMakerFeeBtc = OfferUtil.isCurrencyForMakerFeeBtc(preferences, bsqWalletService, amount);
Coin makerFeeAsCoin = offerUtil.getMakerFee(amount);
boolean isCurrencyForMakerFeeBtc = offerUtil.isCurrencyForMakerFeeBtc(amount);
Coin buyerSecurityDepositAsCoin = getBuyerSecurityDeposit(amount, buyerSecurityDepositAsDouble);
Coin sellerSecurityDepositAsCoin = getSellerSecurityDeposit(amount, sellerSecurityDeposit);
long maxTradeLimit = getMaxTradeLimit(paymentAccount, currencyCode, direction);
long maxTradeLimit = offerUtil.getMaxTradeLimit(paymentAccount, currencyCode, direction);
long maxTradePeriod = paymentAccount.getMaxTradePeriod();

// reserved for future use cases
Expand All @@ -200,15 +180,11 @@ public Offer createAndGetOffer(String offerId,
long lowerClosePrice = 0;
long upperClosePrice = 0;
String hashOfChallenge = null;
Map<String, String> extraDataMap = OfferUtil.getExtraDataMap(accountAgeWitnessService,
referralIdService,
paymentAccount,
Map<String, String> extraDataMap = offerUtil.getExtraDataMap(paymentAccount,
currencyCode,
preferences,
direction);

OfferUtil.validateOfferData(filterManager,
p2PService,
offerUtil.validateOfferData(
buyerSecurityDepositAsDouble,
paymentAccount,
currencyCode,
Expand Down Expand Up @@ -261,8 +237,12 @@ public Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(Coin amount,
OfferPayload.Direction direction,
double buyerSecurityDeposit,
double sellerSecurityDeposit) {
Coin reservedFundsForOffer = getReservedFundsForOffer(direction, amount, buyerSecurityDeposit, sellerSecurityDeposit);
return txFeeEstimationService.getEstimatedFeeAndTxSizeForMaker(reservedFundsForOffer, getMakerFee(amount));
Coin reservedFundsForOffer = getReservedFundsForOffer(direction,
amount,
buyerSecurityDeposit,
sellerSecurityDeposit);
return txFeeEstimationService.getEstimatedFeeAndTxSizeForMaker(reservedFundsForOffer,
offerUtil.getMakerFee(amount));
}

public Coin getReservedFundsForOffer(OfferPayload.Direction direction,
Expand All @@ -274,7 +254,7 @@ public Coin getReservedFundsForOffer(OfferPayload.Direction direction,
amount,
buyerSecurityDeposit,
sellerSecurityDeposit);
if (!isBuyOffer(direction))
if (!offerUtil.isBuyOffer(direction))
reservedFundsForOffer = reservedFundsForOffer.add(amount);

return reservedFundsForOffer;
Expand All @@ -284,7 +264,7 @@ public Coin getSecurityDeposit(OfferPayload.Direction direction,
Coin amount,
double buyerSecurityDeposit,
double sellerSecurityDeposit) {
return isBuyOffer(direction) ?
return offerUtil.isBuyOffer(direction) ?
getBuyerSecurityDeposit(amount, buyerSecurityDeposit) :
getSellerSecurityDeposit(amount, sellerSecurityDeposit);
}
Expand All @@ -294,25 +274,6 @@ public double getSellerSecurityDepositAsDouble(double buyerSecurityDeposit) {
Restrictions.getSellerSecurityDepositAsPercent();
}

public Coin getMakerFee(Coin amount) {
return makerFeeProvider.getMakerFee(bsqWalletService, preferences, amount);
}

public long getMaxTradeLimit(PaymentAccount paymentAccount,
String currencyCode,
OfferPayload.Direction direction) {
if (paymentAccount != null) {
return accountAgeWitnessService.getMyTradeLimit(paymentAccount, currencyCode, direction);
} else {
return 0;
}
}

public boolean isBuyOffer(OfferPayload.Direction direction) {
return OfferUtil.isBuyOffer(direction);
}


///////////////////////////////////////////////////////////////////////////////////////////
// Private
///////////////////////////////////////////////////////////////////////////////////////////
Expand All @@ -322,20 +283,13 @@ private boolean isMarketPriceAvailable(String currencyCode) {
return marketPrice != null && marketPrice.isExternallyProvidedPrice();
}

private boolean isHalCashAccount(PaymentAccount paymentAccount) {
return paymentAccount instanceof HalCashAccount;
}

private Coin getBuyerSecurityDeposit(Coin amount, double buyerSecurityDeposit) {
Coin percentOfAmountAsCoin = CoinUtil.getPercentOfAmountAsCoin(buyerSecurityDeposit, amount);
return getBoundedBuyerSecurityDeposit(percentOfAmountAsCoin);
}

private Coin getSellerSecurityDeposit(Coin amount, double sellerSecurityDeposit) {
Coin amountAsCoin = amount;
if (amountAsCoin == null)
amountAsCoin = Coin.ZERO;

Coin amountAsCoin = (amount == null) ? Coin.ZERO : amount;
Coin percentOfAmountAsCoin = CoinUtil.getPercentOfAmountAsCoin(sellerSecurityDeposit, amountAsCoin);
return getBoundedSellerSecurityDeposit(percentOfAmountAsCoin);
}
Expand Down
29 changes: 0 additions & 29 deletions core/src/main/java/bisq/core/offer/MakerFeeProvider.java
View file

This file was deleted.

9 changes: 5 additions & 4 deletions core/src/main/java/bisq/core/offer/Offer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import bisq.core.payment.payload.PaymentMethod;
import bisq.core.provider.price.MarketPrice;
import bisq.core.provider.price.PriceFeedService;
import bisq.core.util.VolumeUtil;

import bisq.network.p2p.NodeAddress;

Expand Down Expand Up @@ -96,13 +97,13 @@ public enum State {
private final OfferPayload offerPayload;
@JsonExclude
@Getter
transient private ObjectProperty<Offer.State> stateProperty = new SimpleObjectProperty<>(Offer.State.UNKNOWN);
final transient private ObjectProperty<Offer.State> stateProperty = new SimpleObjectProperty<>(Offer.State.UNKNOWN);
@JsonExclude
@Nullable
transient private OfferAvailabilityProtocol availabilityProtocol;
@JsonExclude
@Getter
transient private StringProperty errorMessageProperty = new SimpleStringProperty();
final transient private StringProperty errorMessageProperty = new SimpleStringProperty();
@JsonExclude
@Nullable
@Setter
Expand Down Expand Up @@ -231,9 +232,9 @@ public Volume getVolumeByAmount(Coin amount) {
if (price != null && amount != null) {
Volume volumeByAmount = price.getVolumeByAmount(amount);
if (offerPayload.getPaymentMethodId().equals(PaymentMethod.HAL_CASH_ID))
volumeByAmount = OfferUtil.getAdjustedVolumeForHalCash(volumeByAmount);
volumeByAmount = VolumeUtil.getAdjustedVolumeForHalCash(volumeByAmount);
else if (CurrencyUtil.isFiatCurrency(offerPayload.getCurrencyCode()))
volumeByAmount = OfferUtil.getRoundedFiatVolume(volumeByAmount);
volumeByAmount = VolumeUtil.getRoundedFiatVolume(volumeByAmount);

return volumeByAmount;
} else {
Expand Down
Loading

0 comments on commit ab6be23

Please sign in to comment.