accurate tx fee estimation based on weight

core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java

@@ -23,7 +23,6 @@ import bisq.core.trade.TradeManager;
 import bisq.core.trade.HavenoUtils;
 import bisq.core.util.ParsingUtils;
 
-import com.google.common.collect.TreeMultimap;
 import com.google.common.util.concurrent.Service.State;
 import com.google.inject.name.Named;
 import common.utils.JsonUtils;
@@ -31,7 +30,6 @@ import java.io.File;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -50,7 +48,7 @@ import monero.common.MoneroRpcConnection;
 import monero.common.MoneroRpcError;
 import monero.common.MoneroUtils;
 import monero.daemon.MoneroDaemonRpc;
-import monero.daemon.model.MoneroKeyImageSpentStatus;
+import monero.daemon.model.MoneroFeeEstimate;
 import monero.daemon.model.MoneroNetworkType;
 import monero.daemon.model.MoneroOutput;
 import monero.daemon.model.MoneroSubmitTxResult;
@@ -87,6 +85,8 @@ public class XmrWalletService {
     private static final String MONERO_WALLET_RPC_DEFAULT_PASSWORD = "password"; // only used if account password is null
     private static final String MONERO_WALLET_NAME = "haveno_XMR";
     private static final String MONERO_MULTISIG_WALLET_PREFIX = "xmr_multisig_trade_";
+    private static final int MINER_FEE_PADDING_MULTIPLIER = 2; // extra padding for miner fees = estimated fee * multiplier
+    private static final double MINER_FEE_TOLERANCE = 0.25; // miner fee must be within percent of expected fee
 
     private final CoreAccountService accountService;
     private final CoreMoneroConnectionsService connectionsService;
@@ -265,32 +265,40 @@ public class XmrWalletService {
      * to the sender's payout address. Additional funds are reserved to allow
      * fluctuations in the mining fee.
      *
-     * @param tradeFee is the trade fee
+     * @param tradeFee - trade fee
-     * @param depositAmount the amount needed for the trade minus the trade fee
+     * @param depositAmount - amount needed for the trade minus the trade fee
+     * @param returnAddress - return address for deposit amount
+     * @param addPadding - reserve extra padding for miner fee fluctuations
      * @return a transaction to reserve a trade
      */
-    public MoneroTxWallet createReserveTx(BigInteger tradeFee, String returnAddress, BigInteger depositAmount, boolean freezeInputs) {
+    public MoneroTxWallet createReserveTx(BigInteger tradeFee, String returnAddress, BigInteger depositAmount, boolean addPadding) {
         MoneroWallet wallet = getWallet();
         synchronized (wallet) {
 
-            // get expected mining fee
+            // add miner fee padding to deposit amount
-            MoneroTxWallet miningFeeTx = wallet.createTx(new MoneroTxConfig()
+            if (addPadding) {
-                    .setAccountIndex(0)
+
-                    .addDestination(HavenoUtils.getTradeFeeAddress(), tradeFee)
+                // get expected mining fee
-                    .addDestination(returnAddress, depositAmount));
+                MoneroTxWallet feeEstimateTx = wallet.createTx(new MoneroTxConfig()
-            BigInteger miningFee = miningFeeTx.getFee();
+                        .setAccountIndex(0)
+                        .addDestination(HavenoUtils.getTradeFeeAddress(), tradeFee)
+                        .addDestination(returnAddress, depositAmount));
+                BigInteger feeEstimate = feeEstimateTx.getFee();
+
+                // add extra padding to deposit amount
+                BigInteger minerFeePadding = feeEstimate.multiply(BigInteger.valueOf(MINER_FEE_PADDING_MULTIPLIER));
+                depositAmount = depositAmount.add(minerFeePadding);
+            }
 
             // create reserve tx
             MoneroTxWallet reserveTx = wallet.createTx(new MoneroTxConfig()
-                    .setAccountIndex(0)
+                .setAccountIndex(0)
-                    .addDestination(HavenoUtils.getTradeFeeAddress(), tradeFee)
+                .addDestination(HavenoUtils.getTradeFeeAddress(), tradeFee)
-                    .addDestination(returnAddress, depositAmount.add(miningFee.multiply(BigInteger.valueOf(3l))))); // add thrice the mining fee // TODO (woodser): really require more funds on top of security deposit?
+                .addDestination(returnAddress, depositAmount));
 
             // freeze inputs
-            if (freezeInputs) {
+            for (MoneroOutput input : reserveTx.getInputs()) wallet.freezeOutput(input.getKeyImage().getHex());
-                for (MoneroOutput input : reserveTx.getInputs()) wallet.freezeOutput(input.getKeyImage().getHex());
+            wallet.save();
-                wallet.save();
-            }
 
             return reserveTx;
         }
@@ -368,18 +376,15 @@ public class XmrWalletService {
             if (!check.getReceivedAmount().equals(tradeFee)) throw new RuntimeException("Trade fee is incorrect amount, expected " + tradeFee + " but was " + check.getReceivedAmount());
 
             // verify mining fee
-            BigInteger feeEstimate = getFeeEstimate(txHex);
+            BigInteger feeEstimate = getFeeEstimate(tx.getWeight());
-            BigInteger feeThreshold = feeEstimate.multiply(BigInteger.valueOf(1l)).divide(BigInteger.valueOf(2l)); // must be at least 50% of estimated fee
+            double feeDiff = tx.getFee().subtract(feeEstimate).abs().doubleValue() / feeEstimate.doubleValue();
-            if (tx.getFee().compareTo(feeThreshold) < 0) {
+            if (feeDiff > MINER_FEE_TOLERANCE) throw new Error("Mining fee is not within " + (MINER_FEE_TOLERANCE * 100) + "% of estimated fee, expected " + feeEstimate + " but was " + tx.getFee());
-                throw new RuntimeException("Mining fee is not enough, needed " + feeThreshold + " but was " + tx.getFee());
-            }
 
             // verify deposit amount
             check = wallet.checkTxKey(txHash, txKey, depositAddress);
             if (!check.isGood()) throw new RuntimeException("Invalid proof of deposit amount");
-            BigInteger depositThreshold = depositAmount;
+            if (miningFeePadding) depositAmount = depositAmount.add(feeEstimate.multiply(BigInteger.valueOf(MINER_FEE_PADDING_MULTIPLIER))); // prove reserve of at least deposit amount + miner fee padding
-            if (miningFeePadding) depositThreshold  = depositThreshold.add(feeThreshold.multiply(BigInteger.valueOf(3l))); // prove reserve of at least deposit amount + (3 * min mining fee)
+            if (check.getReceivedAmount().compareTo(depositAmount) < 0) throw new RuntimeException("Deposit amount is not enough, needed " + depositAmount + " but was " + check.getReceivedAmount());
-            if (check.getReceivedAmount().compareTo(depositThreshold) < 0) throw new RuntimeException("Deposit amount is not enough, needed " + depositThreshold + " but was " + check.getReceivedAmount());
         } finally {
             try {
                 daemon.flushTxPool(txHash); // flush tx from pool
@@ -390,9 +395,27 @@ public class XmrWalletService {
         }
     }
 
-    // TODO (woodser): fee estimates are too high, use more accurate estimate
+    /**
-    public BigInteger getFeeEstimate(String txHex) {
+     * Get the tx fee estimate based on its weight.
-        return getDaemon().getFeeEstimate().getFee().multiply(BigInteger.valueOf(txHex.length()));
+     * 
+     * @param txWeight - the tx weight
+     * @return the tx fee estimate
+     */
+    public BigInteger getFeeEstimate(long txWeight) {
+
+        // get fee estimates per kB from daemon
+        MoneroFeeEstimate feeEstimates = getDaemon().getFeeEstimate();
+        BigInteger baseFeeRate = feeEstimates.getFee(); // get normal fee per kB
+        BigInteger qmask = feeEstimates.getQuantizationMask();
+
+        // get tx base fee
+        BigInteger baseFee = baseFeeRate.multiply(BigInteger.valueOf(txWeight));
+
+        // round up to multiple of quantization mask
+        BigInteger[] quotientAndRemainder = baseFee.divideAndRemainder(qmask);
+        BigInteger feeEstimate = qmask.multiply(quotientAndRemainder[0]);
+        if (quotientAndRemainder[1].compareTo(BigInteger.valueOf(0)) > 0) feeEstimate = feeEstimate.add(qmask);
+        return feeEstimate;
     }
 
     public MoneroTx getTx(String txHash) {

core/src/main/java/bisq/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java

@@ -42,16 +42,11 @@ public class MakerReserveOfferFunds extends Task<PlaceOfferModel> {
 
         try {
             runInterceptHook();
-            
+
-            // create tx to estimate fee
+            // create reserve tx with padding
             String returnAddress = model.getXmrWalletService().getOrCreateAddressEntry(offer.getId(), XmrAddressEntry.Context.TRADE_PAYOUT).getAddressString();
             BigInteger makerFee = ParsingUtils.coinToAtomicUnits(offer.getMakerFee());
             BigInteger depositAmount = ParsingUtils.coinToAtomicUnits(model.getReservedFundsForOffer());
-            MoneroTxWallet feeEstimateTx = model.getXmrWalletService().createReserveTx(makerFee, returnAddress, depositAmount, false);
-
-            // create reserve tx and freeze inputs
-            BigInteger feeEstimate = model.getXmrWalletService().getFeeEstimate(feeEstimateTx.getFullHex());
-            depositAmount = depositAmount.add(feeEstimate.multiply(BigInteger.valueOf(3)));
             MoneroTxWallet reserveTx = model.getXmrWalletService().createReserveTx(makerFee, returnAddress, depositAmount, true);
 
             // collect reserved key images // TODO (woodser): switch to proof of reserve?

core/src/main/java/bisq/core/trade/Trade.java

@@ -716,9 +716,9 @@ public abstract class Trade implements Tradable, Model {
      */
     public MoneroTxWallet createPayoutTx() {
 
-        // gather relevant info
+        // gather info
         XmrWalletService walletService = processModel.getProvider().getXmrWalletService();
-        MoneroWallet multisigWallet = walletService.getMultisigWallet(this.getId());
+        MoneroWallet multisigWallet = getWallet();
         if (multisigWallet.isMultisigImportNeeded()) throw new RuntimeException("Cannot create payout tx because multisig import is needed");
         String sellerPayoutAddress = this.getSeller().getPayoutAddressString();
         String buyerPayoutAddress = this.getBuyer().getPayoutAddressString();

core/src/main/java/bisq/core/trade/protocol/tasks/TakerReserveTradeFunds.java

@@ -38,18 +38,13 @@ public class TakerReserveTradeFunds extends TradeTask {
         try {
             runInterceptHook();
 
-            // create tx to estimate fee
+            // create reserve tx without padding
             String returnAddress = model.getXmrWalletService().getOrCreateAddressEntry(trade.getOffer().getId(), XmrAddressEntry.Context.TRADE_PAYOUT).getAddressString();
             BigInteger takerFee = ParsingUtils.coinToAtomicUnits(trade.getTakerFee());
             BigInteger depositAmount = ParsingUtils.centinerosToAtomicUnits(processModel.getFundsNeededForTradeAsLong());
-            MoneroTxWallet feeEstimateTx = model.getXmrWalletService().createReserveTx(takerFee, returnAddress, depositAmount, false);
-
-            // create reserve tx and freeze inputs
-            BigInteger feeEstimate = model.getXmrWalletService().getFeeEstimate(feeEstimateTx.getFullHex());
-            depositAmount = depositAmount.add(feeEstimate.multiply(BigInteger.valueOf(3)));
             MoneroTxWallet reserveTx = model.getXmrWalletService().createReserveTx(takerFee, returnAddress, depositAmount, true);
 
-            // collect reserved key images // TODO (woodser): switch to proof of reserve?
+            // collect reserved key images
             List<String> reservedKeyImages = new ArrayList<String>();
             for (MoneroOutput input : reserveTx.getInputs()) reservedKeyImages.add(input.getKeyImage().getHex());