From d876cc14fc3b7143b9d2da08dae180856415473b Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Fri, 11 Jan 2019 23:20:18 +0100
Subject: [PATCH 01/13] Add TxFeeEstimationService

---
 .../bisq/core/btc/TxFeeEstimationService.java | 159 ++++++++++++++++++
 1 file changed, 159 insertions(+)
 create mode 100644 core/src/main/java/bisq/core/btc/TxFeeEstimationService.java

diff --git a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
new file mode 100644
index 00000000000..2b84a7a08c5
--- /dev/null
+++ b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
@@ -0,0 +1,159 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.btc;
+
+import bisq.core.btc.wallet.BtcWalletService;
+import bisq.core.provider.fee.FeeService;
+import bisq.core.user.Preferences;
+
+import bisq.common.util.Tuple2;
+
+import org.bitcoinj.core.Coin;
+import org.bitcoinj.core.InsufficientMoneyException;
+
+import javax.inject.Inject;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import java.util.List;
+
+import lombok.extern.slf4j.Slf4j;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Util class for getting the estimated tx fee for maker or taker fee tx.
+ */
+@Slf4j
+public class TxFeeEstimationService {
+    public static int TYPICAL_TX_WITH_1_INPUT_SIZE = 260;
+    private final FeeService feeService;
+    private final BtcWalletService btcWalletService;
+    private final Preferences preferences;
+
+    @Inject
+    public TxFeeEstimationService(FeeService feeService,
+                                  BtcWalletService btcWalletService,
+                                  Preferences preferences) {
+
+        this.feeService = feeService;
+        this.btcWalletService = btcWalletService;
+        this.preferences = preferences;
+    }
+
+    public Tuple2<Coin, Integer> getEstimatedFeeAndTxSizeForTaker(Coin fundsNeededForTrade, Coin tradeFee) {
+        return getEstimatedFeeAndTxSize(true,
+                fundsNeededForTrade,
+                tradeFee,
+                feeService,
+                btcWalletService,
+                preferences);
+    }
+
+    public Tuple2<Coin, Integer> getEstimatedFeeAndTxSizeForMaker(Coin reservedFundsForOffer,
+                                                                  Coin tradeFee) {
+        return getEstimatedFeeAndTxSize(false,
+                reservedFundsForOffer,
+                tradeFee,
+                feeService,
+                btcWalletService,
+                preferences);
+    }
+
+    private Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(boolean isTaker,
+                                                           Coin amount,
+                                                           Coin tradeFee,
+                                                           FeeService feeService,
+                                                           BtcWalletService btcWalletService,
+                                                           Preferences preferences) {
+        Coin txFeePerByte = feeService.getTxFeePerByte();
+        // We start with min taker fee size of 260
+        int estimatedTxSize = 260;
+        try {
+            estimatedTxSize = getEstimatedTxSize(List.of(tradeFee, amount), estimatedTxSize, txFeePerByte, btcWalletService);
+        } catch (InsufficientMoneyException e) {
+            if (isTaker) {
+                // if we cannot do the estimation we use the payout tx size
+                estimatedTxSize = 380;
+            }
+            log.info("We cannot do the fee estimation because there are not enough funds in the wallet. This is expected " +
+                    "if the user pays from an external wallet. In that case we use an estimated tx size of {} bytes.", estimatedTxSize);
+        }
+
+        if (!preferences.isPayFeeInBtc()) {
+            // If we pay the fee in BSQ we have one input more which adds about 150 bytes
+            estimatedTxSize += 150;
+        }
+
+        Coin txFee;
+        int size;
+        if (isTaker) {
+            int averageSize = (estimatedTxSize + 320) / 2;  // deposit tx has about 320 bytes
+            // We use at least the size of the payout tx to not underpay at payout.
+            size = Math.max(380, averageSize);
+            txFee = txFeePerByte.multiply(size);
+            log.info("Fee estimation resulted in a tx size of {} bytes.\n" +
+                    "We use an average between the taker fee tx and the deposit tx (320 bytes) which results in {} bytes.\n" +
+                    "The payout tx has 380 bytes, we use that as our min value. Size for fee calculation is {} bytes.\n" +
+                    "The tx fee of {} Sat", estimatedTxSize, averageSize, size, txFee.value);
+        } else {
+            size = estimatedTxSize;
+            txFee = txFeePerByte.multiply(size);
+            log.info("Fee estimation resulted in a tx size of {} bytes and a tx fee of {} Sat.", size, txFee.value);
+        }
+
+        return new Tuple2<>(txFee, size);
+    }
+
+    @VisibleForTesting
+    static int getEstimatedTxSize(List<Coin> outputValues,
+                                  int initialEstimatedTxSize,
+                                  Coin txFeePerByte,
+                                  BtcWalletService btcWalletService)
+            throws InsufficientMoneyException {
+        boolean isInTolerance;
+        int estimatedTxSize = initialEstimatedTxSize;
+        int realTxSize;
+        int counter = 0;
+        do {
+            Coin txFee = txFeePerByte.multiply(estimatedTxSize);
+            realTxSize = btcWalletService.getEstimatedFeeTxSize(outputValues, txFee);
+            isInTolerance = isInTolerance(estimatedTxSize, realTxSize, 0.2);
+            if (!isInTolerance) {
+                estimatedTxSize = realTxSize;
+            }
+            counter++;
+        }
+        while (!isInTolerance && counter < 10);
+        if (!isInTolerance) {
+            log.warn("We could not find a tx which satisfies our tolerance requirement of 20%. " +
+                            "realTxSize={}, estimatedTxSize={}",
+                    realTxSize, estimatedTxSize);
+        }
+        return estimatedTxSize;
+    }
+
+    @VisibleForTesting
+    static boolean isInTolerance(int estimatedSize, int txSize, double tolerance) {
+        checkArgument(estimatedSize > 0, "estimatedSize must be positive");
+        checkArgument(txSize > 0, "txSize must be positive");
+        checkArgument(tolerance > 0, "tolerance must be positive");
+        double deviation = Math.abs(1 - ((double) estimatedSize / (double) txSize));
+        return deviation <= tolerance;
+    }
+}

From 76dd6b5126eb451021e8e3a9554c6357060cc934 Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Fri, 11 Jan 2019 23:20:46 +0100
Subject: [PATCH 02/13] Add TxFeeEstimationServiceTest

---
 .../core/btc/TxFeeEstimationServiceTest.java  | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java

diff --git a/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java b/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
new file mode 100644
index 00000000000..70fd45b512f
--- /dev/null
+++ b/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
@@ -0,0 +1,123 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.btc;
+
+import bisq.core.btc.wallet.BtcWalletService;
+
+import org.bitcoinj.core.Coin;
+import org.bitcoinj.core.InsufficientMoneyException;
+
+import java.util.List;
+
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(BtcWalletService.class)
+@PowerMockIgnore({"com.sun.org.apache.xerces.*", "javax.xml.*", "org.xml.*"})
+public class TxFeeEstimationServiceTest {
+
+    @Test
+    public void testGetEstimatedTxSize() throws InsufficientMoneyException {
+        List<Coin> outputValues = List.of(Coin.valueOf(2000), Coin.valueOf(3000));
+        int initialEstimatedTxSize;
+        Coin txFeePerByte;
+        BtcWalletService btcWalletService = mock(BtcWalletService.class);
+        int result;
+        int realTxSize;
+        Coin txFee;
+
+        initialEstimatedTxSize = 260;
+        txFeePerByte = Coin.valueOf(10);
+        realTxSize = 260;
+
+        txFee = txFeePerByte.multiply(initialEstimatedTxSize);
+        when(btcWalletService.getEstimatedFeeTxSize(outputValues, txFee)).thenReturn(realTxSize);
+        result = TxFeeEstimationService.getEstimatedTxSize(outputValues, initialEstimatedTxSize, txFeePerByte, btcWalletService);
+        assertEquals(260, result);
+
+
+        // TODO check how to use the mocking framework for repeated calls
+        // The btcWalletService.getEstimatedFeeTxSize returns 0 at repeated calls in the while loop....
+       /* initialEstimatedTxSize = 260;
+        txFeePerByte = Coin.valueOf(10);
+        realTxSize = 2600;
+
+        txFee = txFeePerByte.multiply(initialEstimatedTxSize);
+        when(btcWalletService.getEstimatedFeeTxSize(outputValues, txFee)).thenReturn(realTxSize);
+        result = TxFeeEstimationService.getEstimatedTxSize(outputValues, initialEstimatedTxSize, txFeePerByte, btcWalletService);
+        assertEquals(2600, result);
+
+        initialEstimatedTxSize = 2600;
+        txFeePerByte = Coin.valueOf(10);
+        realTxSize = 260;
+
+        txFee = txFeePerByte.multiply(initialEstimatedTxSize);
+        when(btcWalletService.getEstimatedFeeTxSize(outputValues, txFee)).thenReturn(realTxSize);
+        result = TxFeeEstimationService.getEstimatedTxSize(outputValues, initialEstimatedTxSize, txFeePerByte, btcWalletService);
+        assertEquals(260, result);*/
+    }
+
+    @Test
+    public void testIsInTolerance() {
+        int estimatedSize;
+        int txSize;
+        double tolerance;
+        boolean result;
+
+        estimatedSize = 100;
+        txSize = 100;
+        tolerance = 0.0001;
+        result = TxFeeEstimationService.isInTolerance(estimatedSize, txSize, tolerance);
+        assertTrue(result);
+
+        estimatedSize = 100;
+        txSize = 200;
+        tolerance = 0.2;
+        result = TxFeeEstimationService.isInTolerance(estimatedSize, txSize, tolerance);
+        assertFalse(result);
+
+        estimatedSize = 120;
+        txSize = 100;
+        tolerance = 0.2;
+        result = TxFeeEstimationService.isInTolerance(estimatedSize, txSize, tolerance);
+        assertTrue(result);
+
+        estimatedSize = 200;
+        txSize = 100;
+        tolerance = 1;
+        result = TxFeeEstimationService.isInTolerance(estimatedSize, txSize, tolerance);
+        assertTrue(result);
+
+        estimatedSize = 201;
+        txSize = 100;
+        tolerance = 1;
+        result = TxFeeEstimationService.isInTolerance(estimatedSize, txSize, tolerance);
+        assertFalse(result);
+    }
+}

From 274beef3e578a708ecae5fa930b2e62fdb0f0ebd Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Fri, 11 Jan 2019 23:21:06 +0100
Subject: [PATCH 03/13] Add getEstimatedFeeTxSize method

---
 .../bisq/core/btc/wallet/BtcWalletService.java | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java
index d00ac34d923..5760df2abf9 100644
--- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java
+++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java
@@ -957,6 +957,24 @@ private boolean feeEstimationNotSatisfied(int counter, Transaction tx) {
                         tx.getFee().value - targetFee > 1000);
     }
 
+    public int getEstimatedFeeTxSize(List<Coin> outputValues, Coin txFee)
+            throws InsufficientMoneyException, AddressFormatException {
+        Transaction transaction = new Transaction(params);
+        Address dummyAddress = wallet.currentReceiveKey().toAddress(params);
+        outputValues.forEach(outputValue -> transaction.addOutput(outputValue, dummyAddress));
+
+        SendRequest sendRequest = SendRequest.forTx(transaction);
+        sendRequest.shuffleOutputs = false;
+        sendRequest.aesKey = aesKey;
+        sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
+        sendRequest.fee = txFee;
+        sendRequest.feePerKb = Coin.ZERO;
+        sendRequest.ensureMinRequiredFee = false;
+        sendRequest.changeAddress = dummyAddress;
+        wallet.completeTx(sendRequest);
+        return transaction.bitcoinSerialize().length;
+    }
+
 
     ///////////////////////////////////////////////////////////////////////////////////////////
     // Withdrawal Send

From ba114cbdd0e968feb22b20efe49e4c73603a989a Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Fri, 11 Jan 2019 23:21:17 +0100
Subject: [PATCH 04/13] Remove estimateBtcTradingFeeTxSize

---
 .../core/btc/wallet/TradeWalletService.java   | 31 -------------------
 1 file changed, 31 deletions(-)

diff --git a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
index cc41d021959..69a5162568a 100644
--- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
+++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
@@ -230,37 +230,6 @@ public Transaction createBtcTradingFeeTx(Address fundingAddress,
         }
     }
 
-    public Transaction estimateBtcTradingFeeTxSize(Address fundingAddress,
-                                                   Address reservedForTradeAddress,
-                                                   Address changeAddress,
-                                                   Coin reservedFundsForOffer,
-                                                   boolean useSavingsWallet,
-                                                   Coin tradingFee,
-                                                   Coin txFee,
-                                                   String feeReceiverAddresses)
-            throws InsufficientMoneyException, AddressFormatException {
-        Transaction tradingFeeTx = new Transaction(params);
-        tradingFeeTx.addOutput(tradingFee, Address.fromBase58(params, feeReceiverAddresses));
-        tradingFeeTx.addOutput(reservedFundsForOffer, reservedForTradeAddress);
-
-        SendRequest sendRequest = SendRequest.forTx(tradingFeeTx);
-        sendRequest.shuffleOutputs = false;
-        sendRequest.aesKey = aesKey;
-        if (useSavingsWallet)
-            sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
-        else
-            sendRequest.coinSelector = new BtcCoinSelector(fundingAddress);
-
-        sendRequest.fee = txFee;
-        sendRequest.feePerKb = Coin.ZERO;
-        sendRequest.ensureMinRequiredFee = false;
-        sendRequest.changeAddress = changeAddress;
-        checkNotNull(wallet, "Wallet must not be null");
-        log.info("estimateBtcTradingFeeTxSize");
-        wallet.completeTx(sendRequest);
-        return tradingFeeTx;
-    }
-
     public Transaction completeBsqTradingFeeTx(Transaction preparedBsqTx,
                                                Address fundingAddress,
                                                Address reservedForTradeAddress,

From d09296e86c0fdca360abc9fef112c20a48beaed5 Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Fri, 11 Jan 2019 23:23:30 +0100
Subject: [PATCH 05/13] Use TxFeeEstimationService

- feeTxSizeEstimationRecursionCounter was never reset, so that caused
 probably bugs by ignoring fee estimation after it was called >10 times.
 The default size was used then so the bug was not very obvious as long
 the tx size was not very large.
---
 .../java/bisq/core/btc/BitcoinModule.java     |   1 +
 .../main/offer/MutableOfferDataModel.java     |  85 +++-------
 .../createoffer/CreateOfferDataModel.java     |   6 +-
 .../offer/takeoffer/TakeOfferDataModel.java   | 145 ++++++++----------
 .../editoffer/EditOfferDataModel.java         |   6 +-
 5 files changed, 95 insertions(+), 148 deletions(-)

diff --git a/core/src/main/java/bisq/core/btc/BitcoinModule.java b/core/src/main/java/bisq/core/btc/BitcoinModule.java
index b9e365a3bbd..69c6d3bc128 100644
--- a/core/src/main/java/bisq/core/btc/BitcoinModule.java
+++ b/core/src/main/java/bisq/core/btc/BitcoinModule.java
@@ -86,6 +86,7 @@ protected void configure() {
         bind(FeeProvider.class).in(Singleton.class);
         bind(PriceFeedService.class).in(Singleton.class);
         bind(FeeService.class).in(Singleton.class);
+        bind(TxFeeEstimationService.class).in(Singleton.class);
     }
 }
 
diff --git a/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java b/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
index 755f4aa3a8e..642a73f2935 100644
--- a/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
@@ -19,13 +19,13 @@
 
 import bisq.core.app.BisqEnvironment;
 import bisq.core.arbitration.Arbitrator;
+import bisq.core.btc.TxFeeEstimationService;
 import bisq.core.btc.listeners.BalanceListener;
 import bisq.core.btc.listeners.BsqBalanceListener;
 import bisq.core.btc.model.AddressEntry;
 import bisq.core.btc.wallet.BsqWalletService;
 import bisq.core.btc.wallet.BtcWalletService;
 import bisq.core.btc.wallet.Restrictions;
-import bisq.core.btc.wallet.TradeWalletService;
 import bisq.core.filter.FilterManager;
 import bisq.core.locale.CurrencyUtil;
 import bisq.core.locale.Res;
@@ -58,11 +58,10 @@
 
 import bisq.common.app.Version;
 import bisq.common.crypto.KeyRing;
+import bisq.common.util.Tuple2;
 import bisq.common.util.Utilities;
 
-import org.bitcoinj.core.Address;
 import org.bitcoinj.core.Coin;
-import org.bitcoinj.core.InsufficientMoneyException;
 import org.bitcoinj.core.Transaction;
 
 import com.google.inject.Inject;
@@ -106,8 +105,8 @@ public abstract class MutableOfferDataModel extends OfferDataModel implements Bs
     final String shortOfferId;
     private final FilterManager filterManager;
     private final AccountAgeWitnessService accountAgeWitnessService;
-    private final TradeWalletService tradeWalletService;
     private final FeeService feeService;
+    private final TxFeeEstimationService txFeeEstimationService;
     private final ReferralIdService referralIdService;
     private final BSFormatter btcFormatter;
     private final String offerId;
@@ -135,10 +134,9 @@ public abstract class MutableOfferDataModel extends OfferDataModel implements Bs
     protected PaymentAccount paymentAccount;
     protected boolean isTabSelected;
     protected double marketPriceMargin = 0;
-    private Coin txFeeFromFeeService = Coin.ZERO;
-    private boolean marketPriceAvailable;
-    private int feeTxSize = 260; // size of typical tx with 1 input
-    private int feeTxSizeEstimationRecursionCounter;
+    protected Coin txFeeFromFeeService = Coin.ZERO;
+    protected boolean marketPriceAvailable;
+    protected int feeTxSize = TxFeeEstimationService.TYPICAL_TX_WITH_1_INPUT_SIZE;
     protected boolean allowAmountUpdate = true;
 
 
@@ -147,11 +145,19 @@ public abstract class MutableOfferDataModel extends OfferDataModel implements Bs
     ///////////////////////////////////////////////////////////////////////////////////////////
 
     @Inject
-    public MutableOfferDataModel(OpenOfferManager openOfferManager, BtcWalletService btcWalletService, BsqWalletService bsqWalletService,
-                                 Preferences preferences, User user, KeyRing keyRing, P2PService p2PService,
-                                 PriceFeedService priceFeedService, FilterManager filterManager,
-                                 AccountAgeWitnessService accountAgeWitnessService, TradeWalletService tradeWalletService,
-                                 FeeService feeService, ReferralIdService referralIdService,
+    public MutableOfferDataModel(OpenOfferManager openOfferManager,
+                                 BtcWalletService btcWalletService,
+                                 BsqWalletService bsqWalletService,
+                                 Preferences preferences,
+                                 User user,
+                                 KeyRing keyRing,
+                                 P2PService p2PService,
+                                 PriceFeedService priceFeedService,
+                                 FilterManager filterManager,
+                                 AccountAgeWitnessService accountAgeWitnessService,
+                                 FeeService feeService,
+                                 TxFeeEstimationService txFeeEstimationService,
+                                 ReferralIdService referralIdService,
                                  BSFormatter btcFormatter) {
         super(btcWalletService);
 
@@ -164,8 +170,8 @@ public MutableOfferDataModel(OpenOfferManager openOfferManager, BtcWalletService
         this.priceFeedService = priceFeedService;
         this.filterManager = filterManager;
         this.accountAgeWitnessService = accountAgeWitnessService;
-        this.tradeWalletService = tradeWalletService;
         this.feeService = feeService;
+        this.txFeeEstimationService = txFeeEstimationService;
         this.referralIdService = referralIdService;
         this.btcFormatter = btcFormatter;
 
@@ -438,57 +444,14 @@ Offer createAndGetOffer() {
 
     // This works only if we have already funds in the wallet
     public void estimateTxSize() {
-        txFeeFromFeeService = feeService.getTxFee(feeTxSize);
-        Address fundingAddress = btcWalletService.getFreshAddressEntry().getAddress();
-        Address reservedForTradeAddress = btcWalletService.getOrCreateAddressEntry(offerId, AddressEntry.Context.RESERVED_FOR_TRADE).getAddress();
-        Address changeAddress = btcWalletService.getFreshAddressEntry().getAddress();
-
         Coin reservedFundsForOffer = getSecurityDeposit();
         if (!isBuyOffer())
             reservedFundsForOffer = reservedFundsForOffer.add(amount.get());
 
-        checkNotNull(user.getAcceptedArbitrators(), "user.getAcceptedArbitrators() must not be null");
-        checkArgument(!user.getAcceptedArbitrators().isEmpty(), "user.getAcceptedArbitrators() must not be empty");
-        String dummyArbitratorAddress = user.getAcceptedArbitrators().get(0).getBtcAddress();
-        try {
-            log.info("We create a dummy tx to see if our estimated size is in the accepted range. feeTxSize={}," +
-                            " txFee based on feeTxSize: {}, recommended txFee is {} sat/byte",
-                    feeTxSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-            Transaction tradeFeeTx = tradeWalletService.estimateBtcTradingFeeTxSize(
-                    fundingAddress,
-                    reservedForTradeAddress,
-                    changeAddress,
-                    reservedFundsForOffer,
-                    true,
-                    getMakerFee(),
-                    txFeeFromFeeService,
-                    dummyArbitratorAddress);
-
-            final int txSize = tradeFeeTx.bitcoinSerialize().length;
-            // use feeTxSizeEstimationRecursionCounter to avoid risk for endless loop
-            if (txSize > feeTxSize * 1.2 && feeTxSizeEstimationRecursionCounter < 10) {
-                feeTxSizeEstimationRecursionCounter++;
-                log.info("txSize is {} bytes but feeTxSize used for txFee calculation was {} bytes. We try again with an " +
-                        "adjusted txFee to reach the target tx fee.", txSize, feeTxSize);
-                feeTxSize = txSize;
-                txFeeFromFeeService = feeService.getTxFee(feeTxSize);
-                // lets try again with the adjusted txSize and fee.
-                estimateTxSize();
-            } else {
-                log.info("feeTxSize {} bytes", feeTxSize);
-                log.info("txFee based on estimated size: {}, recommended txFee is {} sat/byte",
-                        txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-            }
-        } catch (InsufficientMoneyException e) {
-            // If we need to fund from an external wallet we can assume we only have 1 input (260 bytes).
-            log.warn("We cannot do the fee estimation because there are not enough funds in the wallet. This is expected " +
-                    "if the user pays from an external wallet. In that case we use an estimated tx size of 260 bytes.");
-            feeTxSize = 260;
-            txFeeFromFeeService = feeService.getTxFee(feeTxSize);
-            log.info("feeTxSize {} bytes", feeTxSize);
-            log.info("txFee based on estimated size: {}, recommended txFee is {} sat/byte",
-                    txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-        }
+        Tuple2<Coin, Integer> estimatedFeeAndTxSize = txFeeEstimationService.getEstimatedFeeAndTxSizeForMaker(reservedFundsForOffer,
+                getMakerFee());
+        txFeeFromFeeService = estimatedFeeAndTxSize.first;
+        feeTxSize = estimatedFeeAndTxSize.second;
     }
 
     void onPlaceOffer(Offer offer, TransactionResultHandler resultHandler) {
diff --git a/desktop/src/main/java/bisq/desktop/main/offer/createoffer/CreateOfferDataModel.java b/desktop/src/main/java/bisq/desktop/main/offer/createoffer/CreateOfferDataModel.java
index b3f3dd1e2ae..21286166366 100644
--- a/desktop/src/main/java/bisq/desktop/main/offer/createoffer/CreateOfferDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/offer/createoffer/CreateOfferDataModel.java
@@ -23,9 +23,9 @@
 
 import bisq.desktop.main.offer.MutableOfferDataModel;
 
+import bisq.core.btc.TxFeeEstimationService;
 import bisq.core.btc.wallet.BsqWalletService;
 import bisq.core.btc.wallet.BtcWalletService;
-import bisq.core.btc.wallet.TradeWalletService;
 import bisq.core.filter.FilterManager;
 import bisq.core.offer.OpenOfferManager;
 import bisq.core.payment.AccountAgeWitnessService;
@@ -60,8 +60,8 @@ public CreateOfferDataModel(OpenOfferManager openOfferManager,
                                 PriceFeedService priceFeedService,
                                 FilterManager filterManager,
                                 AccountAgeWitnessService accountAgeWitnessService,
-                                TradeWalletService tradeWalletService,
                                 FeeService feeService,
+                                TxFeeEstimationService txFeeEstimationService,
                                 ReferralIdService referralIdService,
                                 BSFormatter btcFormatter) {
         super(openOfferManager,
@@ -74,8 +74,8 @@ public CreateOfferDataModel(OpenOfferManager openOfferManager,
                 priceFeedService,
                 filterManager,
                 accountAgeWitnessService,
-                tradeWalletService,
                 feeService,
+                txFeeEstimationService,
                 referralIdService,
                 btcFormatter);
     }
diff --git a/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java b/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java
index 64a35a7995e..fd505455b6b 100644
--- a/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java
@@ -21,12 +21,12 @@
 import bisq.desktop.main.overlays.popups.Popup;
 
 import bisq.core.arbitration.Arbitrator;
+import bisq.core.btc.TxFeeEstimationService;
 import bisq.core.btc.listeners.BalanceListener;
 import bisq.core.btc.model.AddressEntry;
 import bisq.core.btc.wallet.BsqWalletService;
 import bisq.core.btc.wallet.BtcWalletService;
 import bisq.core.btc.wallet.Restrictions;
-import bisq.core.btc.wallet.TradeWalletService;
 import bisq.core.filter.FilterManager;
 import bisq.core.locale.CurrencyUtil;
 import bisq.core.locale.Res;
@@ -48,9 +48,9 @@
 import bisq.core.user.User;
 import bisq.core.util.CoinUtil;
 
-import org.bitcoinj.core.Address;
+import bisq.common.util.Tuple2;
+
 import org.bitcoinj.core.Coin;
-import org.bitcoinj.core.InsufficientMoneyException;
 import org.bitcoinj.core.Transaction;
 import org.bitcoinj.wallet.Wallet;
 
@@ -65,6 +65,8 @@
 import java.util.List;
 import java.util.Set;
 
+import org.jetbrains.annotations.NotNull;
+
 import javax.annotation.Nullable;
 
 import static com.google.common.base.Preconditions.checkArgument;
@@ -82,8 +84,8 @@ class TakeOfferDataModel extends OfferDataModel {
     private final FeeService feeService;
     private final FilterManager filterManager;
     private final Preferences preferences;
+    private final TxFeeEstimationService txFeeEstimationService;
     private final PriceFeedService priceFeedService;
-    private final TradeWalletService tradeWalletService;
     private final AccountAgeWitnessService accountAgeWitnessService;
 
     private Coin txFeeFromFeeService;
@@ -103,7 +105,6 @@ class TakeOfferDataModel extends OfferDataModel {
     Price tradePrice;
     // 260 kb is size of typical trade fee tx with 1 input but trade tx (deposit and payout) are larger so we adjust to 320
     private int feeTxSize = 320;
-    private int feeTxSizeEstimationRecursionCounter;
     private boolean freezeFee;
     private Coin txFeePerByteFromFeeService;
 
@@ -115,9 +116,13 @@ class TakeOfferDataModel extends OfferDataModel {
 
     @Inject
     TakeOfferDataModel(TradeManager tradeManager,
-                       BtcWalletService btcWalletService, BsqWalletService bsqWalletService,
-                       User user, FeeService feeService, FilterManager filterManager,
-                       Preferences preferences, PriceFeedService priceFeedService, TradeWalletService tradeWalletService,
+                       BtcWalletService btcWalletService,
+                       BsqWalletService bsqWalletService,
+                       User user, FeeService feeService,
+                       FilterManager filterManager,
+                       Preferences preferences,
+                       TxFeeEstimationService txFeeEstimationService,
+                       PriceFeedService priceFeedService,
                        AccountAgeWitnessService accountAgeWitnessService) {
         super(btcWalletService);
 
@@ -127,8 +132,8 @@ class TakeOfferDataModel extends OfferDataModel {
         this.feeService = feeService;
         this.filterManager = filterManager;
         this.preferences = preferences;
+        this.txFeeEstimationService = txFeeEstimationService;
         this.priceFeedService = priceFeedService;
-        this.tradeWalletService = tradeWalletService;
         this.accountAgeWitnessService = accountAgeWitnessService;
 
         // isMainNet.set(preferences.getBaseCryptoNetwork() == BitcoinNetwork.BTC_MAINNET);
@@ -287,7 +292,7 @@ void onTakeOffer(TradeResultHandler tradeResultHandler) {
         checkNotNull(txFeeFromFeeService, "txFeeFromFeeService must not be null");
         checkNotNull(getTakerFee(), "takerFee must not be null");
 
-        Coin fundsNeededForTrade = getSecurityDeposit().add(txFeeFromFeeService).add(txFeeFromFeeService);
+        Coin fundsNeededForTrade = getFundsNeededForTrade();
         if (isBuyOffer())
             fundsNeededForTrade = fundsNeededForTrade.add(amount.get());
 
@@ -328,81 +333,37 @@ void onTakeOffer(TradeResultHandler tradeResultHandler) {
     // leading to a smaller tx and too high fees. Simply updating the fee estimation would lead to changed required funds
     // and if funds get higher (if tx get larger) the user would get confused (adding small inputs would increase total required funds).
     // So that would require more thoughts how to deal with all those cases.
-    private void estimateTxSize() {
-        Address fundingAddress = btcWalletService.getFreshAddressEntry().getAddress();
+    public void estimateTxSize() {
         int txSize = 0;
         if (btcWalletService.getBalance(Wallet.BalanceType.AVAILABLE).isPositive()) {
-            txFeeFromFeeService = getTxFeeBySize(feeTxSize);
-
-            Address reservedForTradeAddress = btcWalletService.getOrCreateAddressEntry(offer.getId(), AddressEntry.Context.RESERVED_FOR_TRADE).getAddress();
-            Address changeAddress = btcWalletService.getFreshAddressEntry().getAddress();
-
-            Coin reservedFundsForOffer = getSecurityDeposit().add(txFeeFromFeeService).add(txFeeFromFeeService);
+            Coin fundsNeededForTrade = getFundsNeededForTrade();
             if (isBuyOffer())
-                reservedFundsForOffer = reservedFundsForOffer.add(amount.get());
-
-            checkNotNull(user.getAcceptedArbitrators(), "user.getAcceptedArbitrators() must not be null");
-            checkArgument(!user.getAcceptedArbitrators().isEmpty(), "user.getAcceptedArbitrators() must not be empty");
-            String dummyArbitratorAddress = user.getAcceptedArbitrators().get(0).getBtcAddress();
-            try {
-                log.debug("We create a dummy tx to see if our estimated size is in the accepted range. feeTxSize={}," +
-                                " txFee based on feeTxSize: {}, recommended txFee is {} sat/byte",
-                        feeTxSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-                Transaction tradeFeeTx = tradeWalletService.estimateBtcTradingFeeTxSize(
-                        fundingAddress,
-                        reservedForTradeAddress,
-                        changeAddress,
-                        reservedFundsForOffer,
-                        true,
-                        getTakerFee(),
-                        txFeeFromFeeService,
-                        dummyArbitratorAddress);
-
-                txSize = tradeFeeTx.bitcoinSerialize().length;
-                // use feeTxSizeEstimationRecursionCounter to avoid risk for endless loop
-                // We use the tx size for the trade fee tx as target for the fees.
-                // The deposit and payout txs are determined +/- 1 output but the trade fee tx can have either 1 or many inputs
-                // so we need to make sure the trade fee tx gets the correct fee to not get stuck.
-                // We use a 20% tolerance frm out default 320 byte size (typical for deposit and payout) and only if we get a
-                // larger size we increase the fee. Worst case is that we overpay for the other follow up txs, but better than
-                // use a too low fee and get stuck.
-                if (txSize > feeTxSize * 1.2 && feeTxSizeEstimationRecursionCounter < 10) {
-                    feeTxSizeEstimationRecursionCounter++;
-                    log.info("txSize is {} bytes but feeTxSize used for txFee calculation was {} bytes. We try again with an " +
-                            "adjusted txFee to reach the target tx fee.", txSize, feeTxSize);
-
-                    feeTxSize = txSize;
-                    txFeeFromFeeService = getTxFeeBySize(txSize);
-
-                    // lets try again with the adjusted txSize and fee.
-                    estimateTxSize();
-                } else {
-                    // We are done with estimation iterations
-                    if (feeTxSizeEstimationRecursionCounter < 10)
-                        log.info("Fee estimation completed:\n" +
-                                        "txFee based on estimated size of {} bytes. Average tx size = {} bytes. Actual tx size = {} bytes. TxFee is {} ({} sat/byte)",
-                                feeTxSize, getAverageSize(feeTxSize), txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-                    else
-                        log.warn("We could not estimate the fee as the feeTxSizeEstimationRecursionCounter exceeded our limit of 10 recursions.\n" +
-                                        "txFee based on estimated size of {} bytes. Average tx size = {} bytes. Actual tx size = {} bytes. " +
-                                        "TxFee is {} ({} sat/byte)",
-                                feeTxSize, getAverageSize(feeTxSize), txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-                }
-            } catch (InsufficientMoneyException e) {
-                log.info("We cannot complete the fee estimation because there are not enough funds in the wallet.\n" +
-                                "This is expected if the user has not sufficient funds yet.\n" +
-                                "In that case we use the latest estimated tx size or the default if none has been calculated yet.\n" +
-                                "txFee based on estimated size of {} bytes. Average tx size = {} bytes. Actual tx size = {} bytes. TxFee is {} ({} sat/byte)",
-                        feeTxSize, getAverageSize(feeTxSize), txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
-            }
+                fundsNeededForTrade = fundsNeededForTrade.add(amount.get());
+
+            // As taker we pay 3 times the fee and currently the fee is the same for all 3 txs (trade fee tx, deposit
+            // tx and payout tx).
+            // We should try to change that in future to have the deposit and payout tx with a fixed fee as the size is
+            // there more deterministic.
+            // The trade fee tx can be in the worst case very large if there are many inputs so if we take that tx alone
+            // for the fee estimation we would overpay a lot.
+            // On the other side if we have the best case of a 1 input tx fee tx then it is only 260 bytes but the
+            // other 2 txs are larger (320 and 380 bytes) and would get a lower fee/byte as intended.
+            // We apply following model to not overpay too much but be on the safe side as well.
+            // We sum the taker fee tx and the deposit tx together as it can be assumed that both be in the same block and
+            // as they are dependent txs the miner will pick both if the fee in total is good enough.
+            // We make sure that the fee is sufficient to meet our intended fee/byte for the larger payout tx with 380 bytes.
+            Tuple2<Coin, Integer> estimatedFeeAndTxSize = txFeeEstimationService.getEstimatedFeeAndTxSizeForTaker(fundsNeededForTrade,
+                    getTakerFee());
+            txFeeFromFeeService = estimatedFeeAndTxSize.first;
+            feeTxSize = estimatedFeeAndTxSize.second;
         } else {
-            feeTxSize = 320;
-            txFeeFromFeeService = getTxFeeBySize(feeTxSize);
+            feeTxSize = 380;
+            txFeeFromFeeService = txFeePerByteFromFeeService.multiply(feeTxSize);
             log.info("We cannot do the fee estimation because there are no funds in the wallet.\nThis is expected " +
                             "if the user has not funded his wallet yet.\n" +
-                            "In that case we use an estimated tx size of 320 bytes.\n" +
-                            "txFee based on estimated size of {} bytes. Average tx size = {} bytes. Actual tx size = {} bytes. TxFee is {} ({} sat/byte)",
-                    feeTxSize, getAverageSize(feeTxSize), txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
+                            "In that case we use an estimated tx size of 380 bytes.\n" +
+                            "txFee based on estimated size of {} bytes. feeTxSize = {} bytes. Actual tx size = {} bytes. TxFee is {} ({} sat/byte)",
+                    feeTxSize, feeTxSize, txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
         }
     }
 
@@ -620,10 +581,32 @@ public String getCurrencyNameAndCode() {
     }
 
     public Coin getTotalTxFee() {
+        Coin totalTxTees = txFeeFromFeeService.add(getTxFeeForDepositTx()).add(getTxFeeForPayoutTx());
         if (isCurrencyForTakerFeeBtc())
-            return txFeeFromFeeService.multiply(3);
+            return totalTxTees;
         else
-            return txFeeFromFeeService.multiply(3).subtract(getTakerFee() != null ? getTakerFee() : Coin.ZERO);
+            return totalTxTees.subtract(getTakerFee() != null ? getTakerFee() : Coin.ZERO);
+    }
+
+    @NotNull
+    private Coin getFundsNeededForTrade() {
+        return getSecurityDeposit().add(getTxFeeForDepositTx()).add(getTxFeeForPayoutTx());
+    }
+
+    private Coin getTxFeeForDepositTx() {
+        //TODO fix with new trade protocol!
+        // Unfortunately we cannot change that to the correct fees as it would break backward compatibility
+        // We still might find a way with offer version or app version checks so lets keep that commented out
+        // code as that shows how it should be.
+        return txFeeFromFeeService; //feeService.getTxFee(320);
+    }
+
+    private Coin getTxFeeForPayoutTx() {
+        //TODO fix with new trade protocol!
+        // Unfortunately we cannot change that to the correct fees as it would break backward compatibility
+        // We still might find a way with offer version or app version checks so lets keep that commented out
+        // code as that shows how it should be.
+        return txFeeFromFeeService; //feeService.getTxFee(380);
     }
 
     public AddressEntry getAddressEntry() {
diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/editoffer/EditOfferDataModel.java b/desktop/src/main/java/bisq/desktop/main/portfolio/editoffer/EditOfferDataModel.java
index 2054869aa06..c6cbc56f3fa 100644
--- a/desktop/src/main/java/bisq/desktop/main/portfolio/editoffer/EditOfferDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/portfolio/editoffer/EditOfferDataModel.java
@@ -20,9 +20,9 @@
 
 import bisq.desktop.main.offer.MutableOfferDataModel;
 
+import bisq.core.btc.TxFeeEstimationService;
 import bisq.core.btc.wallet.BsqWalletService;
 import bisq.core.btc.wallet.BtcWalletService;
-import bisq.core.btc.wallet.TradeWalletService;
 import bisq.core.filter.FilterManager;
 import bisq.core.locale.CurrencyUtil;
 import bisq.core.locale.TradeCurrency;
@@ -67,8 +67,8 @@ class EditOfferDataModel extends MutableOfferDataModel {
                        PriceFeedService priceFeedService,
                        FilterManager filterManager,
                        AccountAgeWitnessService accountAgeWitnessService,
-                       TradeWalletService tradeWalletService,
                        FeeService feeService,
+                       TxFeeEstimationService txFeeEstimationService,
                        ReferralIdService referralIdService,
                        BSFormatter btcFormatter,
                        CorePersistenceProtoResolver corePersistenceProtoResolver) {
@@ -82,8 +82,8 @@ class EditOfferDataModel extends MutableOfferDataModel {
                 priceFeedService,
                 filterManager,
                 accountAgeWitnessService,
-                tradeWalletService,
                 feeService,
+                txFeeEstimationService,
                 referralIdService,
                 btcFormatter);
         this.corePersistenceProtoResolver = corePersistenceProtoResolver;

From 28d0db8ac8c1ad775904e945fcfa6c0fbde0a876 Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Sat, 12 Jan 2019 00:16:57 +0100
Subject: [PATCH 06/13] Fix missing param in test class

---
 .../main/offer/createoffer/CreateOfferViewModelTest.java    | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/desktop/src/test/java/bisq/desktop/main/offer/createoffer/CreateOfferViewModelTest.java b/desktop/src/test/java/bisq/desktop/main/offer/createoffer/CreateOfferViewModelTest.java
index d38f32ed5cc..d63836a444c 100644
--- a/desktop/src/test/java/bisq/desktop/main/offer/createoffer/CreateOfferViewModelTest.java
+++ b/desktop/src/test/java/bisq/desktop/main/offer/createoffer/CreateOfferViewModelTest.java
@@ -22,6 +22,7 @@
 import bisq.desktop.util.validation.FiatPriceValidator;
 import bisq.desktop.util.validation.SecurityDepositValidator;
 
+import bisq.core.btc.TxFeeEstimationService;
 import bisq.core.btc.model.AddressEntry;
 import bisq.core.btc.wallet.BsqWalletService;
 import bisq.core.btc.wallet.BtcWalletService;
@@ -70,7 +71,7 @@
 @PrepareForTest({BtcWalletService.class, AddressEntry.class, PriceFeedService.class, User.class,
         FeeService.class, CreateOfferDataModel.class, PaymentAccount.class, BsqWalletService.class,
         SecurityDepositValidator.class, AccountAgeWitnessService.class, BsqFormatter.class, Preferences.class,
-        BsqWalletService.class})
+        BsqWalletService.class, TxFeeEstimationService.class})
 @PowerMockIgnore({"com.sun.org.apache.xerces.*", "javax.xml.*", "org.xml.*"})
 public class CreateOfferViewModelTest {
 
@@ -98,6 +99,7 @@ public void setUp() {
         BsqWalletService bsqWalletService = mock(BsqWalletService.class);
         SecurityDepositValidator securityDepositValidator = mock(SecurityDepositValidator.class);
         AccountAgeWitnessService accountAgeWitnessService = mock(AccountAgeWitnessService.class);
+        TxFeeEstimationService txFeeEstimationService = mock(TxFeeEstimationService.class);
 
         when(btcWalletService.getOrCreateAddressEntry(anyString(), any())).thenReturn(addressEntry);
         when(btcWalletService.getBalanceForAddress(any())).thenReturn(Coin.valueOf(1000L));
@@ -112,7 +114,7 @@ public void setUp() {
         when(bsqFormatter.formatCoin(any())).thenReturn("0");
         when(bsqWalletService.getAvailableBalance()).thenReturn(Coin.ZERO);
 
-        CreateOfferDataModel dataModel = new CreateOfferDataModel(null, btcWalletService, bsqWalletService, empty, user, null, null, priceFeedService, null, accountAgeWitnessService, null, feeService, null, bsFormatter);
+        CreateOfferDataModel dataModel = new CreateOfferDataModel(null, btcWalletService, bsqWalletService, empty, user, null, null, priceFeedService, null, accountAgeWitnessService, feeService, txFeeEstimationService, null, bsFormatter);
         dataModel.initWithData(OfferPayload.Direction.BUY, new CryptoCurrency("BTC", "bitcoin"));
         dataModel.activate();
 

From 645719cf64960a4b24a56f8b75d874debab6d91f Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Mon, 14 Jan 2019 14:33:40 +0100
Subject: [PATCH 07/13] Remove PowerMockRunner

---
 .../test/java/bisq/core/btc/TxFeeEstimationServiceTest.java    | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java b/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
index 70fd45b512f..90b5109608c 100644
--- a/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
+++ b/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
@@ -26,10 +26,8 @@
 
 import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
 
 import org.junit.Test;
-import org.junit.runner.RunWith;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -37,7 +35,6 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-@RunWith(PowerMockRunner.class)
 @PrepareForTest(BtcWalletService.class)
 @PowerMockIgnore({"com.sun.org.apache.xerces.*", "javax.xml.*", "org.xml.*"})
 public class TxFeeEstimationServiceTest {

From 461dd1d02b1666a027276eb8289cf890b39c2deb Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Mon, 14 Jan 2019 15:08:22 +0100
Subject: [PATCH 08/13] Use constants for tx size values

---
 .../bisq/core/btc/TxFeeEstimationService.java    | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
index 2b84a7a08c5..9368545b325 100644
--- a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
+++ b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
@@ -42,6 +42,10 @@
 @Slf4j
 public class TxFeeEstimationService {
     public static int TYPICAL_TX_WITH_1_INPUT_SIZE = 260;
+    private static int DEPOSIT_TX_SIZE = 320;
+    private static int PAYOUT_TX_SIZE = 380;
+    private static int BSQ_INPUT_INCREASE = 150;
+    private static int MAX_ITERATIONS = 10;
     private final FeeService feeService;
     private final BtcWalletService btcWalletService;
     private final Preferences preferences;
@@ -83,13 +87,13 @@ private Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(boolean isTaker,
                                                            Preferences preferences) {
         Coin txFeePerByte = feeService.getTxFeePerByte();
         // We start with min taker fee size of 260
-        int estimatedTxSize = 260;
+        int estimatedTxSize = TYPICAL_TX_WITH_1_INPUT_SIZE;
         try {
             estimatedTxSize = getEstimatedTxSize(List.of(tradeFee, amount), estimatedTxSize, txFeePerByte, btcWalletService);
         } catch (InsufficientMoneyException e) {
             if (isTaker) {
                 // if we cannot do the estimation we use the payout tx size
-                estimatedTxSize = 380;
+                estimatedTxSize = PAYOUT_TX_SIZE;
             }
             log.info("We cannot do the fee estimation because there are not enough funds in the wallet. This is expected " +
                     "if the user pays from an external wallet. In that case we use an estimated tx size of {} bytes.", estimatedTxSize);
@@ -97,15 +101,15 @@ private Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(boolean isTaker,
 
         if (!preferences.isPayFeeInBtc()) {
             // If we pay the fee in BSQ we have one input more which adds about 150 bytes
-            estimatedTxSize += 150;
+            estimatedTxSize += BSQ_INPUT_INCREASE;
         }
 
         Coin txFee;
         int size;
         if (isTaker) {
-            int averageSize = (estimatedTxSize + 320) / 2;  // deposit tx has about 320 bytes
+            int averageSize = (estimatedTxSize + DEPOSIT_TX_SIZE) / 2;  // deposit tx has about 320 bytes
             // We use at least the size of the payout tx to not underpay at payout.
-            size = Math.max(380, averageSize);
+            size = Math.max(PAYOUT_TX_SIZE, averageSize);
             txFee = txFeePerByte.multiply(size);
             log.info("Fee estimation resulted in a tx size of {} bytes.\n" +
                     "We use an average between the taker fee tx and the deposit tx (320 bytes) which results in {} bytes.\n" +
@@ -139,7 +143,7 @@ static int getEstimatedTxSize(List<Coin> outputValues,
             }
             counter++;
         }
-        while (!isInTolerance && counter < 10);
+        while (!isInTolerance && counter < MAX_ITERATIONS);
         if (!isInTolerance) {
             log.warn("We could not find a tx which satisfies our tolerance requirement of 20%. " +
                             "realTxSize={}, estimatedTxSize={}",

From 877d0ae3acb2a1ea94e389f99cbd0194ebae6aee Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Mon, 14 Jan 2019 15:09:37 +0100
Subject: [PATCH 09/13] Break up test method

Set ignore to failing tests

@Bernard could you have a look and try to get those working?
Reason for problem is that repeated calls to getEstimatedFeeTxSize
do not work (returns 0 at second call in loop which cause test to fail)
---
 .../core/btc/TxFeeEstimationServiceTest.java  | 36 ++++++++++++++++---
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java b/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
index 90b5109608c..2f9d6b36d41 100644
--- a/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
+++ b/core/src/test/java/bisq/core/btc/TxFeeEstimationServiceTest.java
@@ -27,6 +27,7 @@
 import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -40,7 +41,7 @@
 public class TxFeeEstimationServiceTest {
 
     @Test
-    public void testGetEstimatedTxSize() throws InsufficientMoneyException {
+    public void testGetEstimatedTxSize_withDefaultTxSize() throws InsufficientMoneyException {
         List<Coin> outputValues = List.of(Coin.valueOf(2000), Coin.valueOf(3000));
         int initialEstimatedTxSize;
         Coin txFeePerByte;
@@ -57,18 +58,43 @@ public void testGetEstimatedTxSize() throws InsufficientMoneyException {
         when(btcWalletService.getEstimatedFeeTxSize(outputValues, txFee)).thenReturn(realTxSize);
         result = TxFeeEstimationService.getEstimatedTxSize(outputValues, initialEstimatedTxSize, txFeePerByte, btcWalletService);
         assertEquals(260, result);
+    }
 
+    // FIXME @Bernard could you have a look?
+    @Test
+    @Ignore
+    public void testGetEstimatedTxSize_withLargeTx() throws InsufficientMoneyException {
+        List<Coin> outputValues = List.of(Coin.valueOf(2000), Coin.valueOf(3000));
+        int initialEstimatedTxSize;
+        Coin txFeePerByte;
+        BtcWalletService btcWalletService = mock(BtcWalletService.class);
+        int result;
+        int realTxSize;
+        Coin txFee;
 
-        // TODO check how to use the mocking framework for repeated calls
-        // The btcWalletService.getEstimatedFeeTxSize returns 0 at repeated calls in the while loop....
-       /* initialEstimatedTxSize = 260;
+        initialEstimatedTxSize = 260;
         txFeePerByte = Coin.valueOf(10);
         realTxSize = 2600;
 
         txFee = txFeePerByte.multiply(initialEstimatedTxSize);
         when(btcWalletService.getEstimatedFeeTxSize(outputValues, txFee)).thenReturn(realTxSize);
+
+        // repeated calls to getEstimatedFeeTxSize do not work (returns 0 at second call in loop which cause test to fail)
         result = TxFeeEstimationService.getEstimatedTxSize(outputValues, initialEstimatedTxSize, txFeePerByte, btcWalletService);
         assertEquals(2600, result);
+    }
+
+    // FIXME @Bernard could you have a look?
+    @Test
+    @Ignore
+    public void testGetEstimatedTxSize_withSmallTx() throws InsufficientMoneyException {
+        List<Coin> outputValues = List.of(Coin.valueOf(2000), Coin.valueOf(3000));
+        int initialEstimatedTxSize;
+        Coin txFeePerByte;
+        BtcWalletService btcWalletService = mock(BtcWalletService.class);
+        int result;
+        int realTxSize;
+        Coin txFee;
 
         initialEstimatedTxSize = 2600;
         txFeePerByte = Coin.valueOf(10);
@@ -77,7 +103,7 @@ public void testGetEstimatedTxSize() throws InsufficientMoneyException {
         txFee = txFeePerByte.multiply(initialEstimatedTxSize);
         when(btcWalletService.getEstimatedFeeTxSize(outputValues, txFee)).thenReturn(realTxSize);
         result = TxFeeEstimationService.getEstimatedTxSize(outputValues, initialEstimatedTxSize, txFeePerByte, btcWalletService);
-        assertEquals(260, result);*/
+        assertEquals(260, result);
     }
 
     @Test

From ed8df87a9b09825f5a47e06dad2c9a49d503b51a Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Fri, 18 Jan 2019 21:47:16 +0100
Subject: [PATCH 10/13] Add comment to give more background about the fee
 estimation

---
 .../bisq/core/btc/TxFeeEstimationService.java    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
index 9368545b325..c0b066c2182 100644
--- a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
+++ b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
@@ -46,6 +46,7 @@ public class TxFeeEstimationService {
     private static int PAYOUT_TX_SIZE = 380;
     private static int BSQ_INPUT_INCREASE = 150;
     private static int MAX_ITERATIONS = 10;
+
     private final FeeService feeService;
     private final BtcWalletService btcWalletService;
     private final Preferences preferences;
@@ -101,6 +102,7 @@ private Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(boolean isTaker,
 
         if (!preferences.isPayFeeInBtc()) {
             // If we pay the fee in BSQ we have one input more which adds about 150 bytes
+            // TODO: Clarify if there is always just one additional input or if there can be more.
             estimatedTxSize += BSQ_INPUT_INCREASE;
         }
 
@@ -124,6 +126,20 @@ private Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(boolean isTaker,
         return new Tuple2<>(txFee, size);
     }
 
+    // We start with the initialEstimatedTxSize for a tx with 1 input (260) bytes and get from BitcoinJ a tx back which
+    // contains the required inputs to fund that tx (out outputs + miner fee). The miner fee is that case is based on
+    // the assumption that we only need 1 input. Once we receive back the real tx size from the tx BitcoinJ has created
+    // with the required inputs we compare that if it is in the tolerance of 20% from our assumed tx size. If we inside
+    // that tolerance we use that tx size for our fee estimation. If not (if there has been more then 1 inputs) we
+    // apply the new fee based on the reported tx size and request again from BitcoinJ to full that tx with the inputs
+    // to be sufficiently funded. The algorithm how BitcoinJ selects utxos is complex and contains several aspects
+    // (minimize fee, don't create too many tiny utxos,...). We treat that algorithm as an unknown and it is not
+    // guaranteed that there are more inputs required if we increase the fee (it could be that there is a better
+    // selection of inputs chosen if we have increased the fee and therefor less inputs). As the increased fee might
+    // change the number of inputs we need to repeat that process until we are inside of a certain tolerance. To avoid
+    // potential endless loops we add a counter. Worst case would be that the last size we got reported is > 20% off to
+    // the real tx size but as fee estimation is anyway a educated guess in the best case we don't worry too much.
+    // If we have underpaid the tx might take longer to get confirmed.
     @VisibleForTesting
     static int getEstimatedTxSize(List<Coin> outputValues,
                                   int initialEstimatedTxSize,

From 250c7c1e19afdc98cd1fddfff5acef727c2e56e5 Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Wed, 23 Jan 2019 12:32:06 +0100
Subject: [PATCH 11/13] Improve comment

---
 .../java/bisq/core/btc/TxFeeEstimationService.java  | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
index c0b066c2182..9a2e1c76ba5 100644
--- a/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
+++ b/core/src/main/java/bisq/core/btc/TxFeeEstimationService.java
@@ -127,17 +127,18 @@ private Tuple2<Coin, Integer> getEstimatedFeeAndTxSize(boolean isTaker,
     }
 
     // We start with the initialEstimatedTxSize for a tx with 1 input (260) bytes and get from BitcoinJ a tx back which
-    // contains the required inputs to fund that tx (out outputs + miner fee). The miner fee is that case is based on
+    // contains the required inputs to fund that tx (outputs + miner fee). The miner fee in that case is based on
     // the assumption that we only need 1 input. Once we receive back the real tx size from the tx BitcoinJ has created
-    // with the required inputs we compare that if it is in the tolerance of 20% from our assumed tx size. If we inside
-    // that tolerance we use that tx size for our fee estimation. If not (if there has been more then 1 inputs) we
-    // apply the new fee based on the reported tx size and request again from BitcoinJ to full that tx with the inputs
+    // with the required inputs we compare if the size is not more then 20% different to our assumed tx size. If we are inside
+    // that tolerance we use that tx size for our fee estimation, if not (if there has been more then 1 inputs) we
+    // apply the new fee based on the reported tx size and request again from BitcoinJ to fill that tx with the inputs
     // to be sufficiently funded. The algorithm how BitcoinJ selects utxos is complex and contains several aspects
     // (minimize fee, don't create too many tiny utxos,...). We treat that algorithm as an unknown and it is not
     // guaranteed that there are more inputs required if we increase the fee (it could be that there is a better
-    // selection of inputs chosen if we have increased the fee and therefor less inputs). As the increased fee might
+    // selection of inputs chosen if we have increased the fee and therefore less inputs and smaller tx size). As the increased fee might
     // change the number of inputs we need to repeat that process until we are inside of a certain tolerance. To avoid
-    // potential endless loops we add a counter. Worst case would be that the last size we got reported is > 20% off to
+    // potential endless loops we add a counter (we use 10, usually it takes just very few iterations).
+    // Worst case would be that the last size we got reported is > 20% off to
     // the real tx size but as fee estimation is anyway a educated guess in the best case we don't worry too much.
     // If we have underpaid the tx might take longer to get confirmed.
     @VisibleForTesting

From 64f65e01a0451454dcfa2e16f2e2ed7fe9c9ebf1 Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Wed, 23 Jan 2019 12:32:18 +0100
Subject: [PATCH 12/13] Fix typos

---
 .../desktop/main/offer/takeoffer/TakeOfferDataModel.java    | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java b/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java
index fd505455b6b..f6dd5544374 100644
--- a/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferDataModel.java
@@ -581,11 +581,11 @@ public String getCurrencyNameAndCode() {
     }
 
     public Coin getTotalTxFee() {
-        Coin totalTxTees = txFeeFromFeeService.add(getTxFeeForDepositTx()).add(getTxFeeForPayoutTx());
+        Coin totalTxFees = txFeeFromFeeService.add(getTxFeeForDepositTx()).add(getTxFeeForPayoutTx());
         if (isCurrencyForTakerFeeBtc())
-            return totalTxTees;
+            return totalTxFees;
         else
-            return totalTxTees.subtract(getTakerFee() != null ? getTakerFee() : Coin.ZERO);
+            return totalTxFees.subtract(getTakerFee() != null ? getTakerFee() : Coin.ZERO);
     }
 
     @NotNull

From 90e95e5d2e29ae7b6cf813727992151053b75543 Mon Sep 17 00:00:00 2001
From: Manfred Karrer <mk@nucleo.io>
Date: Wed, 23 Jan 2019 12:32:28 +0100
Subject: [PATCH 13/13] Make fields private

---
 .../java/bisq/desktop/main/offer/MutableOfferDataModel.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java b/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
index 642a73f2935..afb033c5911 100644
--- a/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
+++ b/desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
@@ -134,9 +134,9 @@ public abstract class MutableOfferDataModel extends OfferDataModel implements Bs
     protected PaymentAccount paymentAccount;
     protected boolean isTabSelected;
     protected double marketPriceMargin = 0;
-    protected Coin txFeeFromFeeService = Coin.ZERO;
-    protected boolean marketPriceAvailable;
-    protected int feeTxSize = TxFeeEstimationService.TYPICAL_TX_WITH_1_INPUT_SIZE;
+    private Coin txFeeFromFeeService = Coin.ZERO;
+    private boolean marketPriceAvailable;
+    private int feeTxSize = TxFeeEstimationService.TYPICAL_TX_WITH_1_INPUT_SIZE;
     protected boolean allowAmountUpdate = true;