[bisq-network/bisq] [WIP] Fee estimation (#4710)

[chimp1984]

@chimp1984 approved this pull request.

utACK

> @@ -548,6 +577,23 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx,
         return resultTx;
     }
 
+    private Tuple2<Integer, Integer> getNumInputs(Transaction tx) {
+        int numLegacyInputs = 0;
+        int numSegwitInputs = 0;
+        for (TransactionInput input : tx.getInputs()) {
+            TransactionOutput connectedOutput = input.getConnectedOutput();
+            if (connectedOutput == null || ScriptPattern.isP2PKH(connectedOutput.getScriptPubKey()) ||

I assume the motivation for treating the connectedOutput == null as legacyInput is that if we dont know the inputs we use the potentially higher fee estimation. If correct, maybe a comment could help to make it more clear. 

> +//  Size/vsize of typical trade txs
+//  Real txs size/vsize may vary in 1 or 2 bytes from the estimated values.
+//  Values calculated with https://gist.github.com/oscarguindzberg/3d1349cb65d9fd9af9de0feaa3fd27ac
+//  legacy fee tx with 1 input, maker/taker fee paid in btc size/vsize = 258
+//  legacy deposit tx without change size/vsize = 381
+//  legacy deposit tx with change size/vsize = 414
+//  legacy payout tx size/vsize = 337
+//  legacy delayed payout tx size/vsize = 302
+//  segwit fee tx with 1 input, maker/taker fee paid in btc vsize = 173
+//  segwit deposit tx without change vsize = 232
+//  segwit deposit tx with change vsize = 263
+//  segwit payout tx vsize = 169
+//  segwit delayed payout tx vsize = 139
+    public static int TYPICAL_TX_WITH_1_INPUT_SIZE = 175;
+    private static int DEPOSIT_TX_SIZE = 233;
+    private static int PAYOUT_TX_SIZE = 169;

PAYOUT_TX_SIZE is not used anymore and can be deleted.

>          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 = PAYOUT_TX_SIZE;
+                // if we cannot do the estimation we use the deposit tx size

Maybe change comment to: "if we cannot do the estimation we use the largest of our txs which is the deposit tx

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/4710#pullrequestreview-519024523
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20201028/fe23b311/attachment.html>