From 65dba3357d59b1dcdb97b0d9e511b23587752fbc Mon Sep 17 00:00:00 2001 From: Jonathan Chappelow Date: Wed, 7 Dec 2022 15:59:30 -0600 Subject: [PATCH 1/3] client/asset: fix estimateSwap split rejection The enough func in estimateSwap does not account for a split transaction at the start, so it is possible that funding for trySplit would actually choose more UTXOs. Actual order funding accounts for this. For this estimate, we will just not use a split tx if the split-adjusted required funds exceeds the total value of the UTXOs selected with this enough closure. The fix is to reject a split transaction if the output amount plus split tx fees is more than the sum of the amounts of the utxos selected by fund, rather than the total of the available utxos provided to fund initially. There is no point to calling fund again with a different enough func that accounts for the cost split tx because this indicates it would reqiure an additional UTXO, thus making the spit txn wasteful. --- client/asset/btc/btc.go | 26 ++++++++------- client/asset/btc/btc_test.go | 42 +++++++++++------------ client/asset/dcr/dcr.go | 21 +++++++----- client/asset/dcr/dcr_test.go | 64 ++++++++---------------------------- 4 files changed, 60 insertions(+), 93 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 00c5d37e81..92a61e2819 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -1903,15 +1903,19 @@ func (btc *baseWallet) estimateSwap(lots, lotSize, feeSuggestion, maxFeeRate uin bumpedMaxRate := maxFeeRate bumpedNetRate := feeSuggestion if feeBump > 1 { - bumpedMaxRate = uint64(math.Round(float64(bumpedMaxRate) * feeBump)) - bumpedNetRate = uint64(math.Round(float64(bumpedNetRate) * feeBump)) + bumpedMaxRate = uint64(math.Ceil(float64(bumpedMaxRate) * feeBump)) + bumpedNetRate = uint64(math.Ceil(float64(bumpedNetRate) * feeBump)) } val := lots * lotSize - + // This enough func does not account for a split transaction at the start, + // so it is possible that funding for trySplit would actually choose more + // UTXOs. Actual order funding accounts for this. For this estimate, we will + // just not use a split tx if the split-adjusted required funds exceeds the + // total value of the UTXO selected with this enough closure. enough := func(inputsSize, inputsVal uint64) bool { reqFunds := calc.RequiredOrderFundsAlt(val, inputsSize, lots, btc.initTxSizeBase, - btc.initTxSize, bumpedMaxRate) + btc.initTxSize, bumpedMaxRate) // no +splitMaxFees so this is accurate without split return inputsVal >= reqFunds } @@ -1921,7 +1925,7 @@ func (btc *baseWallet) estimateSwap(lots, lotSize, feeSuggestion, maxFeeRate uin } reqFunds := calc.RequiredOrderFundsAlt(val, uint64(inputsSize), lots, - btc.initTxSizeBase, btc.initTxSize, bumpedMaxRate) + btc.initTxSizeBase, btc.initTxSize, bumpedMaxRate) // same as in enough func maxFees := reqFunds - val estHighFunds := calc.RequiredOrderFundsAlt(val, uint64(inputsSize), lots, @@ -1935,23 +1939,21 @@ func (btc *baseWallet) estimateSwap(lots, lotSize, feeSuggestion, maxFeeRate uin } else { estLowFunds += dexbtc.P2SHOutputSize * (lots - 1) * bumpedNetRate } - estLowFees := estLowFunds - val // Math for split transactions is a little different. if trySplit { - _, extraMaxFees := btc.splitBaggageFees(bumpedMaxRate) + _, splitMaxFees := btc.splitBaggageFees(bumpedMaxRate) _, splitFees := btc.splitBaggageFees(bumpedNetRate) - locked := val + maxFees + extraMaxFees - - if avail >= reqFunds+extraMaxFees { + reqTotal := reqFunds + splitMaxFees // ~ rather than actually fund()ing again + if reqTotal <= sum { return &asset.SwapEstimate{ Lots: lots, Value: val, - MaxFees: maxFees + extraMaxFees, + MaxFees: maxFees + splitMaxFees, RealisticBestCase: estLowFees + splitFees, RealisticWorstCase: estHighFees + splitFees, - }, true, locked, nil + }, true, reqFunds, nil // requires reqTotal, but locks reqFunds in the split output } } diff --git a/client/asset/btc/btc_test.go b/client/asset/btc/btc_test.go index fc95215b13..3d5903649f 100644 --- a/client/asset/btc/btc_test.go +++ b/client/asset/btc/btc_test.go @@ -1305,7 +1305,7 @@ func testFundingCoins(t *testing.T, segwit bool, walletType string) { ensureGood() } -func checkMaxOrder(t *testing.T, wallet asset.Wallet, lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { +func checkMaxOrder(t *testing.T, wallet asset.Wallet, lots, swapVal, maxFees, estWorstCase, estBestCase uint64) { t.Helper() maxOrder, err := wallet.MaxOrder(&asset.MaxOrderForm{ LotSize: tLotSize, @@ -1316,10 +1316,10 @@ func checkMaxOrder(t *testing.T, wallet asset.Wallet, lots, swapVal, maxFees, es if err != nil { t.Fatalf("MaxOrder error: %v", err) } - checkSwapEstimate(t, maxOrder, lots, swapVal, maxFees, estWorstCase, estBestCase, locked) + checkSwapEstimate(t, maxOrder, lots, swapVal, maxFees, estWorstCase, estBestCase) } -func checkSwapEstimate(t *testing.T, est *asset.SwapEstimate, lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { +func checkSwapEstimate(t *testing.T, est *asset.SwapEstimate, lots, swapVal, maxFees, estWorstCase, estBestCase uint64) { t.Helper() if est.Lots != lots { t.Fatalf("Estimate has wrong Lots. wanted %d, got %d", lots, est.Lots) @@ -1344,11 +1344,6 @@ func TestFundEdges(t *testing.T) { swapVal := uint64(1e7) lots := swapVal / tLotSize - checkMax := func(lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { - t.Helper() - checkMaxOrder(t, wallet, lots, swapVal, maxFees, estWorstCase, estBestCase, locked) - } - // Base Fees // fee_rate: 34 satoshi / vbyte (MaxFeeRate) // swap_size: 225 bytes (InitTxSize) @@ -1401,8 +1396,9 @@ func TestFundEdges(t *testing.T) { var feeReduction uint64 = swapSize * tBTC.MaxFeeRate estFeeReduction := swapSize * feeSuggestion - checkMax(lots-1, swapVal-tLotSize, backingFees-feeReduction, totalBytes*feeSuggestion-estFeeReduction, - (bestCaseBytes-swapOutputSize)*feeSuggestion, swapVal+backingFees-1) + checkMaxOrder(t, wallet, lots-1, swapVal-tLotSize, backingFees-feeReduction, + totalBytes*feeSuggestion-estFeeReduction, + (bestCaseBytes-swapOutputSize)*feeSuggestion) _, _, err := wallet.FundOrder(ord) if err == nil { @@ -1412,7 +1408,8 @@ func TestFundEdges(t *testing.T) { p2pkhUnspent.Amount = float64(swapVal+backingFees) / 1e8 node.listUnspent = unspents - checkMax(lots, swapVal, backingFees, totalBytes*feeSuggestion, bestCaseBytes*feeSuggestion, swapVal+backingFees) + checkMaxOrder(t, wallet, lots, swapVal, backingFees, totalBytes*feeSuggestion, + bestCaseBytes*feeSuggestion) spendables, _, err := wallet.FundOrder(ord) if err != nil { @@ -1447,7 +1444,9 @@ func TestFundEdges(t *testing.T) { p2pkhUnspent.Amount = float64(v) / 1e8 node.listUnspent = unspents - checkMax(lots, swapVal, backingFees, (totalBytes+splitTxBaggage)*feeSuggestion, (bestCaseBytes+splitTxBaggage)*feeSuggestion, v) + checkMaxOrder(t, wallet, lots, swapVal, backingFees, + (totalBytes+splitTxBaggage)*feeSuggestion, + (bestCaseBytes+splitTxBaggage)*feeSuggestion) coins, _, err = wallet.FundOrder(ord) if err != nil { @@ -1569,11 +1568,6 @@ func TestFundEdgesSegwit(t *testing.T) { swapVal := uint64(1e7) lots := swapVal / tLotSize - checkMax := func(lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { - t.Helper() - checkMaxOrder(t, wallet, lots, swapVal, maxFees, estWorstCase, estBestCase, locked) - } - // Base Fees // fee_rate: 34 satoshi / vbyte (MaxFeeRate) @@ -1625,8 +1619,9 @@ func TestFundEdgesSegwit(t *testing.T) { var feeReduction uint64 = swapSize * tBTC.MaxFeeRate estFeeReduction := swapSize * feeSuggestion - checkMax(lots-1, swapVal-tLotSize, backingFees-feeReduction, totalBytes*feeSuggestion-estFeeReduction, - (bestCaseBytes-swapOutputSize)*feeSuggestion, swapVal+backingFees-1) + checkMaxOrder(t, wallet, lots-1, swapVal-tLotSize, backingFees-feeReduction, + totalBytes*feeSuggestion-estFeeReduction, + (bestCaseBytes-swapOutputSize)*feeSuggestion) _, _, err := wallet.FundOrder(ord) if err == nil { @@ -1636,7 +1631,8 @@ func TestFundEdgesSegwit(t *testing.T) { p2wpkhUnspent.Amount = float64(swapVal+backingFees) / 1e8 node.listUnspent = unspents - checkMax(lots, swapVal, backingFees, totalBytes*feeSuggestion, bestCaseBytes*feeSuggestion, swapVal+backingFees) + checkMaxOrder(t, wallet, lots, swapVal, backingFees, totalBytes*feeSuggestion, + bestCaseBytes*feeSuggestion) spendables, _, err := wallet.FundOrder(ord) if err != nil { @@ -1668,7 +1664,9 @@ func TestFundEdgesSegwit(t *testing.T) { p2wpkhUnspent.Amount = float64(v) / 1e8 node.listUnspent = unspents - checkMax(lots, swapVal, backingFees, (totalBytes+splitTxBaggageSegwit)*feeSuggestion, (bestCaseBytes+splitTxBaggageSegwit)*feeSuggestion, v) + checkMaxOrder(t, wallet, lots, swapVal, backingFees, + (totalBytes+splitTxBaggageSegwit)*feeSuggestion, + (bestCaseBytes+splitTxBaggageSegwit)*feeSuggestion) coins, _, err = wallet.FundOrder(ord) if err != nil { @@ -2904,7 +2902,7 @@ func testPreSwap(t *testing.T, segwit bool, walletType string) { maxFees := totalBytes * tBTC.MaxFeeRate estHighFees := totalBytes * feeSuggestion estLowFees := bestCaseBytes * feeSuggestion - checkSwapEstimate(t, preSwap.Estimate, lots, swapVal, maxFees, estHighFees, estLowFees, minReq) + checkSwapEstimate(t, preSwap.Estimate, lots, swapVal, maxFees, estHighFees, estLowFees) // Too little funding is an error. setFunds(minReq - 1) diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go index 45bafe988e..fb7f159257 100644 --- a/client/asset/dcr/dcr.go +++ b/client/asset/dcr/dcr.go @@ -1185,18 +1185,23 @@ func (dcr *ExchangeWallet) estimateSwap(lots, lotSize, feeSuggestion, maxFeeRate bumpedMaxRate := maxFeeRate bumpedNetRate := feeSuggestion if feeBump > 1 { - bumpedMaxRate = uint64(math.Round(float64(bumpedMaxRate) * feeBump)) - bumpedNetRate = uint64(math.Round(float64(bumpedNetRate) * feeBump)) + bumpedMaxRate = uint64(math.Ceil(float64(bumpedMaxRate) * feeBump)) + bumpedNetRate = uint64(math.Ceil(float64(bumpedNetRate) * feeBump)) } val := lots * lotSize + // The orderEnough func does not account for a split transaction at the + // start, so it is possible that funding for trySplit would actually choose + // more UTXOs. Actual order funding accounts for this. For this estimate, we + // will just not use a split tx if the split-adjusted required funds exceeds + // the total value of the UTXO selected with this enough closure. sum, inputsSize, _, _, _, err := tryFund(utxos, orderEnough(val, lots, bumpedMaxRate)) if err != nil { return nil, false, 0, err } reqFunds := calc.RequiredOrderFundsAlt(val, uint64(inputsSize), lots, - dexdcr.InitTxSizeBase, dexdcr.InitTxSize, bumpedMaxRate) + dexdcr.InitTxSizeBase, dexdcr.InitTxSize, bumpedMaxRate) // as in tryFund's enough func maxFees := reqFunds - val estHighFunds := calc.RequiredOrderFundsAlt(val, uint64(inputsSize), lots, @@ -1210,17 +1215,17 @@ func (dcr *ExchangeWallet) estimateSwap(lots, lotSize, feeSuggestion, maxFeeRate // Math for split transactions is a little different. if trySplit { - extraFees := splitTxBaggage * bumpedMaxRate + splitMaxFees := splitTxBaggage * bumpedMaxRate splitFees := splitTxBaggage * bumpedNetRate - if avail >= reqFunds+extraFees { - locked := val + maxFees + extraFees + reqTotal := reqFunds + splitMaxFees // ~ rather than actually fund()ing again + if reqTotal <= sum { return &asset.SwapEstimate{ Lots: lots, Value: val, - MaxFees: maxFees + extraFees, + MaxFees: maxFees + splitMaxFees, RealisticBestCase: estLowFees + splitFees, RealisticWorstCase: estHighFees + splitFees, - }, true, locked, nil + }, true, reqFunds, nil // requires reqTotal, but locks reqFunds in the split output } } diff --git a/client/asset/dcr/dcr_test.go b/client/asset/dcr/dcr_test.go index 08bb190aa1..0411e08ea3 100644 --- a/client/asset/dcr/dcr_test.go +++ b/client/asset/dcr/dcr_test.go @@ -1101,16 +1101,16 @@ func TestFundingCoins(t *testing.T) { ensureGood() } -func checkMaxOrder(t *testing.T, wallet *ExchangeWallet, lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { +func checkMaxOrder(t *testing.T, wallet *ExchangeWallet, lots, swapVal, maxFees, estWorstCase, estBestCase uint64) { t.Helper() _, maxOrder, err := wallet.maxOrder(tLotSize, feeSuggestion, tDCR.MaxFeeRate) if err != nil { t.Fatalf("MaxOrder error: %v", err) } - checkSwapEstimate(t, maxOrder, lots, swapVal, maxFees, estWorstCase, estBestCase, locked) + checkSwapEstimate(t, maxOrder, lots, swapVal, maxFees, estWorstCase, estBestCase) } -func checkSwapEstimate(t *testing.T, est *asset.SwapEstimate, lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { +func checkSwapEstimate(t *testing.T, est *asset.SwapEstimate, lots, swapVal, maxFees, estWorstCase, estBestCase uint64) { t.Helper() if est.Lots != lots { t.Fatalf("MaxOrder has wrong Lots. wanted %d, got %d", lots, est.Lots) @@ -1136,10 +1136,6 @@ func TestFundEdges(t *testing.T) { swapVal := uint64(1e8) lots := swapVal / tLotSize - checkMax := func(lots, swapVal, maxFees, estWorstCase, estBestCase, locked uint64) { - checkMaxOrder(t, wallet, lots, swapVal, maxFees, estWorstCase, estBestCase, locked) - } - // Swap fees // // fee_rate: 24 atoms / byte (dex MaxFeeRate) @@ -1183,8 +1179,10 @@ func TestFundEdges(t *testing.T) { var feeReduction uint64 = swapSize * tDCR.MaxFeeRate estFeeReduction := swapSize * feeSuggestion - checkMax(lots-1, swapVal-tLotSize, fees-feeReduction, totalBytes*feeSuggestion-estFeeReduction, - (bestCaseBytes-swapOutputSize)*feeSuggestion, swapVal+fees-1) + checkMaxOrder(t, wallet, lots-1, swapVal-tLotSize, + fees-feeReduction, // max fees + totalBytes*feeSuggestion-estFeeReduction, // worst case + (bestCaseBytes-swapOutputSize)*feeSuggestion) // best case _, _, err := wallet.FundOrder(ord) if err == nil { @@ -1194,7 +1192,8 @@ func TestFundEdges(t *testing.T) { p2pkhUnspent.Amount = float64(swapVal+fees) / 1e8 node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent} - checkMax(lots, swapVal, fees, totalBytes*feeSuggestion, bestCaseBytes*feeSuggestion, swapVal+fees) + checkMaxOrder(t, wallet, lots, swapVal, fees, totalBytes*feeSuggestion, + bestCaseBytes*feeSuggestion) _, _, err = wallet.FundOrder(ord) if err != nil { @@ -1223,7 +1222,8 @@ func TestFundEdges(t *testing.T) { v = swapVal + fees node.unspent[0].Amount = float64(v) / 1e8 - checkMax(lots, swapVal, fees, (totalBytes+splitTxBaggage)*feeSuggestion, (bestCaseBytes+splitTxBaggage)*feeSuggestion, v) + checkMaxOrder(t, wallet, lots, swapVal, fees, (totalBytes+splitTxBaggage)*feeSuggestion, + (bestCaseBytes+splitTxBaggage)*feeSuggestion) coins, _, err = wallet.FundOrder(ord) if err != nil { @@ -1252,47 +1252,9 @@ func TestFundEdges(t *testing.T) { if err != nil { t.Fatalf("error fixing split tx: %v", err) } - - // TODO: test version mismatch - wallet.config().useSplitTx = false - // TODO: fix the p2sh test so that the redeem script is a p2pk pkScript or a - // multisig pkScript, not a p2pkh pkScript. - - // P2SH pkScript size = 23 bytes - // P2PK pkScript (the redeem script) = 35 bytes - // P2SH redeem input size = overhead(58) + sigScript((1 + 73 + 1) + (1 + 33 + 1)) + - // p2sh pkScript length(23) = 191 bytes vs 166 for P2PKH - // backing_fees: 191 bytes * fee_rate(24) = 4584 atoms - // total bytes: 2344 + 191 = 2535 - // total: 56256 + 4584 = 60840 atoms - // OR (2344 + 191) * 24 = 60840 - // p2shRedeem, _ := hex.DecodeString("76a914" + "db1755408acd315baa75c18ebbe0e8eaddf64a97" + "88ac") // (p2pkh! pkScript) 1+1+1+20+1+1 =25 bytes - // scriptAddr := "DcsJEKNF3dQwcSozroei5FRPsbPEmMuWRaj" - // p2shScriptPubKey, _ := hex.DecodeString("a914" + "3ff6a24a50135f69be9ffed744443da08408fc1a" + "87") // 1 + 1 + 20 + 1 = 23 bytes - // fees = 2535 * tDCR.MaxFeeRate - // halfSwap := swapVal / 2 - // p2shUnspent := walletjson.ListUnspentResult{ - // TxID: tTxID, - // Address: scriptAddr, - // Amount: float64(halfSwap) / 1e8, - // Confirmations: 10, - // ScriptPubKey: hex.EncodeToString(p2shScriptPubKey), - // RedeemScript: hex.EncodeToString(p2shRedeem), - // } - // p2pkhUnspent.Amount = float64(halfSwap+fees-1) / 1e8 - // node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent, p2shUnspent} - // _, err = wallet.FundOrder(swapVal, false, tDCR) - // if err == nil { - // t.Fatalf("no error when not enough funds in two utxos") - // } - // p2pkhUnspent.Amount = float64(halfSwap+fees) / 1e8 - // node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent, p2shUnspent} - // _, err = wallet.FundOrder(swapVal, false, tDCR) - // if err != nil { - // t.Fatalf("error when should be enough funding in two utxos: %v", err) - // } + // TODO: test version mismatch } func TestSwap(t *testing.T) { @@ -2474,7 +2436,7 @@ func TestPreSwap(t *testing.T) { maxFees := totalBytes * tDCR.MaxFeeRate estHighFees := totalBytes * feeSuggestion estLowFees := bestCaseBytes * feeSuggestion - checkSwapEstimate(t, preSwap.Estimate, lots, swapVal, maxFees, estHighFees, estLowFees, minReq) + checkSwapEstimate(t, preSwap.Estimate, lots, swapVal, maxFees, estHighFees, estLowFees) // Too little funding is an error. node.unspent[0].Amount = float64(minReq-1) / 1e8 From a07e92e16de252be417a86a95942c6840bc42a3d Mon Sep 17 00:00:00 2001 From: Jonathan Chappelow Date: Wed, 7 Dec 2022 16:01:10 -0600 Subject: [PATCH 2/3] client/asset: show split option when ineffectual with reason --- client/asset/btc/btc.go | 60 +++++++++++++++++++++-------------------- client/asset/dcr/dcr.go | 58 +++++++++++++++++++-------------------- 2 files changed, 60 insertions(+), 58 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 92a61e2819..69dd4f44dd 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -1706,7 +1706,7 @@ func (btc *baseWallet) PreSwap(req *asset.PreSwapForm) (*asset.PreSwap, error) { } // Get the estimate using the current configuration. - est, splitUsed, _, err := btc.estimateSwap(req.Lots, req.LotSize, req.FeeSuggestion, + est, _, _, err := btc.estimateSwap(req.Lots, req.LotSize, req.FeeSuggestion, req.MaxFeeRate, utxos, split, bump) if err != nil { return nil, fmt.Errorf("estimation failed: %v", err) @@ -1714,9 +1714,8 @@ func (btc *baseWallet) PreSwap(req *asset.PreSwapForm) (*asset.PreSwap, error) { var opts []*asset.OrderOption - // If the used split isn't the requested split, the other split option was - // unavailable, so there is no option to offer. - if !req.Immediate && splitUsed == split { + // Only offer the split option for standing orders. + if !req.Immediate { if splitOpt := btc.splitOption(req, utxos, bump); splitOpt != nil { opts = append(opts, splitOpt) } @@ -1853,40 +1852,43 @@ func (btc *baseWallet) splitOption(req *asset.PreSwapForm, utxos []*compositeUTX btc.log.Errorf("estimateSwap (with split) error: %v", err) return nil } - if !splitUsed { - // unable to do the split. no option. - btc.log.Debugf("split option unavailable") - return nil - } symbol := strings.ToUpper(btc.symbol) - xtraFees := splitEst.RealisticWorstCase - noSplitEst.RealisticWorstCase - pctChange := (float64(splitEst.RealisticWorstCase)/float64(noSplitEst.RealisticWorstCase) - 1) * 100 - overlock := noSplitLocked - splitLocked - - var reason string - if pctChange > 1 { - reason = fmt.Sprintf("+%d%% fees, -%s %s overlock", int(math.Round(pctChange)), prettyBTC(overlock), symbol) - } else { - reason = fmt.Sprintf("+%.1f%% fees, -%s %s overlock", pctChange, prettyBTC(overlock), symbol) - } - - desc := fmt.Sprintf("Using a split transaction to prevent temporary overlock of %s %s, but for additional fees of %s %s", - prettyBTC(overlock), symbol, prettyBTC(xtraFees), symbol) - - return &asset.OrderOption{ + opt := &asset.OrderOption{ ConfigOption: asset.ConfigOption{ Key: splitKey, DisplayName: "Pre-size Funds", - Description: desc, - DefaultValue: btc.useSplitTx(), IsBoolean: true, + DefaultValue: false, // not nil interface ShowByDefault: true, }, - Boolean: &asset.BooleanConfig{ - Reason: reason, - }, + Boolean: &asset.BooleanConfig{}, } + + if !splitUsed || splitLocked >= noSplitLocked { // locked check should be redundant + opt.Boolean.Reason = fmt.Sprintf("avoids no %s overlock for this order (ignored)", symbol) + opt.Description = fmt.Sprintf("A split transaction for this order avoids no %s overlock, "+ + "but adds additional fees.", symbol) + return opt // not enabled by default, but explain why + } + + // Since it is usable, apply the user's default value, and set the + // reason and description. + opt.DefaultValue = btc.useSplitTx() + + overlock := noSplitLocked - splitLocked + pctChange := (float64(splitEst.RealisticWorstCase)/float64(noSplitEst.RealisticWorstCase) - 1) * 100 + if pctChange > 1 { + opt.Boolean.Reason = fmt.Sprintf("+%d%% fees, avoids %s %s overlock", int(math.Round(pctChange)), prettyBTC(overlock), symbol) + } else { + opt.Boolean.Reason = fmt.Sprintf("+%.1f%% fees, avoids %s %s overlock", pctChange, prettyBTC(overlock), symbol) + } + + xtraFees := splitEst.RealisticWorstCase - noSplitEst.RealisticWorstCase + opt.Description = fmt.Sprintf("Using a split transaction to prevent temporary overlock of %s %s, but for additional fees of %s %s", + prettyBTC(overlock), symbol, prettyBTC(xtraFees), symbol) + + return opt } // estimateSwap prepares an *asset.SwapEstimate. diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go index fb7f159257..bca7fcd51b 100644 --- a/client/asset/dcr/dcr.go +++ b/client/asset/dcr/dcr.go @@ -1278,7 +1278,7 @@ func (dcr *ExchangeWallet) PreSwap(req *asset.PreSwapForm) (*asset.PreSwap, erro } // Get the estimate for the requested number of lots. - est, splitUsed, _, err := dcr.estimateSwap(req.Lots, req.LotSize, req.FeeSuggestion, + est, _, _, err := dcr.estimateSwap(req.Lots, req.LotSize, req.FeeSuggestion, req.MaxFeeRate, utxos, split, bump) if err != nil { return nil, fmt.Errorf("estimation failed: %v", err) @@ -1286,9 +1286,8 @@ func (dcr *ExchangeWallet) PreSwap(req *asset.PreSwapForm) (*asset.PreSwap, erro var opts []*asset.OrderOption - // If the used split isn't the requested split, the other split option was - // unavailable, so there is no option to offer. - if !req.Immediate && splitUsed == split { + // Only offer the split option for standing orders. + if !req.Immediate { if splitOpt := dcr.splitOption(req, utxos, bump); splitOpt != nil { opts = append(opts, splitOpt) } @@ -1410,40 +1409,41 @@ func (dcr *ExchangeWallet) splitOption(req *asset.PreSwapForm, utxos []*composit dcr.log.Errorf("estimateSwap (with split) error: %v", err) return nil } - if !splitUsed { - // unable to do the split. no option. - dcr.log.Debugf("split option unavailable") - return nil + + opt := &asset.OrderOption{ + ConfigOption: asset.ConfigOption{ + Key: splitKey, + DisplayName: "Pre-size Funds", + IsBoolean: true, + DefaultValue: false, // not nil interface + ShowByDefault: true, + }, + Boolean: &asset.BooleanConfig{}, } - xtraFees := splitEst.RealisticWorstCase - noSplitEst.RealisticWorstCase - pctChange := (float64(splitEst.RealisticWorstCase)/float64(noSplitEst.RealisticWorstCase) - 1) * 100 - overlock := noSplitLocked - splitLocked + if !splitUsed || splitLocked >= noSplitLocked { // locked check should be redundant + opt.Boolean.Reason = "avoids no DCR overlock for this order (ignored)" + opt.Description = "A split transaction for this order avoids no DCR overlock, but adds additional fees." + return opt // not enabled by default, but explain why + } + + // Since it is usable, apply the user's default value, and set the + // reason and description. + opt.DefaultValue = dcr.config().useSplitTx - var reason string + overlock := noSplitLocked - splitLocked + pctChange := (float64(splitEst.RealisticWorstCase)/float64(noSplitEst.RealisticWorstCase) - 1) * 100 if pctChange > 1 { - reason = fmt.Sprintf("+%d%% fees, avoids %s DCR overlock", int(math.Round(pctChange)), amount(overlock)) + opt.Boolean.Reason = fmt.Sprintf("+%d%% fees, avoids %s DCR overlock", int(math.Round(pctChange)), amount(overlock)) } else { - reason = fmt.Sprintf("+%.1f%% fees, avoids %s DCR overlock", pctChange, amount(overlock)) + opt.Boolean.Reason = fmt.Sprintf("+%.1f%% fees, avoids %s DCR overlock", pctChange, amount(overlock)) } - desc := fmt.Sprintf("Using a split transaction to prevent temporary overlock of %s DCR, but for additional fees of %s DCR", + xtraFees := splitEst.RealisticWorstCase - noSplitEst.RealisticWorstCase + opt.Description = fmt.Sprintf("Using a split transaction may prevent temporary overlock of %s DCR, but for additional fees of %s DCR", amount(overlock), amount(xtraFees)) - cfg := dcr.config() - return &asset.OrderOption{ - ConfigOption: asset.ConfigOption{ - Key: splitKey, - DisplayName: "Pre-size Funds", - Description: desc, - DefaultValue: cfg.useSplitTx, - IsBoolean: true, - ShowByDefault: true, - }, - Boolean: &asset.BooleanConfig{ - Reason: reason, - }, - } + return opt } // PreRedeem generates an estimate of the range of redemption fees that could From 5d1d5cf4ddbecb0ef45745261c9ef7b3c047bc9d Mon Sep 17 00:00:00 2001 From: Jonathan Chappelow Date: Wed, 7 Dec 2022 16:27:05 -0600 Subject: [PATCH 3/3] client/core: market orders are immediate (PreOrder) --- client/core/bot.go | 2 +- client/core/core.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/core/bot.go b/client/core/bot.go index 3632d44cec..f0a513a85a 100644 --- a/client/core/bot.go +++ b/client/core/bot.go @@ -1229,7 +1229,7 @@ func (c *Core) feeEstimates(form *TradeForm) (swapFees, redeemFees uint64, err e LotSize: swapLotSize, Lots: lots, MaxFeeRate: fromAsset.MaxFeeRate, - Immediate: (form.IsLimit && form.TifNow), + Immediate: (form.IsLimit && form.TifNow) || !form.IsLimit, FeeSuggestion: swapFeeSuggestion, SelectedOptions: form.Options, RedeemVersion: toWallet.version, diff --git a/client/core/core.go b/client/core/core.go index ae51cc8c51..feb76d93b5 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -5107,7 +5107,7 @@ func (c *Core) PreOrder(form *TradeForm) (*OrderEstimate, error) { LotSize: swapLotSize, Lots: lots, MaxFeeRate: assetConfigs.fromAsset.MaxFeeRate, - Immediate: form.IsLimit && form.TifNow, + Immediate: (form.IsLimit && form.TifNow) || !form.IsLimit, FeeSuggestion: swapFeeSuggestion, SelectedOptions: form.Options, RedeemVersion: assetConfigs.toAsset.Version,