Improve wallet fee logic and fix GUI bugs

morcos commented at 6:18 pm on June 29, 2017: member

This builds on #10589 (first 5 commits from that PR, last 5 commits are new)

The first couple commits refactor to use the CCoinControl class to pass fee calculation parameters around.

This allows for fixing the buggy interaction in QT between the global payTxFee which can be modified by the RPC call settxfee or temporarily modified by the QT custom fee settings. Before these changes the GUI could sometimes send a transaction with a recently set payTxFee and not respect the settings displayed in the GUI. After these changes, using the GUI does not involve the global transaction confirm target or payTxFee.

The prospective fee displays in the smart fee slider and the coin control dialog are changed to use the fee calculation from GetMinimumFee, this simplifies the code and makes them slightly more correct in edge cases.

Maxing the fee calculation with the mempool min fee is move from estimateSmartFee to GetMinimumFee.

This fixes a long standing bug, and should be tagged for 0.15 as it is holding up finalizing the estimatesmartfee RPC API before release.

laanwj added this to the milestone 0.15.0 on Jun 29, 2017

fanquake added the label TX fees and policy on Jun 30, 2017

fanquake added the label Wallet on Jun 30, 2017

in src/wallet/coincontrol.h:29 in 5d0e674ac7 outdated

29-    //! Override the default confirmation target, 0 = use default
30-    int nConfirmTarget;
31+    //! Override the default payTxFee if set, 0 = use smart fee estimation
32+    boost::optional<CFeeRate> m_feerate;
33+    //! Override the default confirmation target
34+    boost::optional<unsigned int> m_confirm_target;

TheBlueMatt commented at 5:02 pm on July 9, 2017:

Gah, I’d really rather keep the old magic value of just 0 means default instead of adding boost::optional (and kinda would prefer to skip boost::option in favor of more descriptive names for m_feerate, too).

morcos commented at 1:04 am on July 10, 2017:

I hate the magic values of 0. Actually I think I should make the logic even slightly more clear. There should just be a clear parameter precedence.

coin_control.m_feerate
coin_control.m_confirm_target
::payTxFee (global)
::nTxConfirmTarget (globa)

It goes down the list until it fines one that is set and all the first 3 should be boost::optional’s. Why do you not like boost::optional? I could skip changing the global payTxFee for now, and still have that work via the 0 magic value. The code to process this will be slightly longer in GetMininumFee but I think it’ll be far more clear no?

TheBlueMatt commented at 1:11 am on July 10, 2017:

Hmm, maybe I shouldnt argue against boost::optional. I’m not a fan of optional types, but they do make sense here. Any further simplification would be nice, this stuff is gross (thanks for cleaning it up!).

in src/wallet/coincontrol.h:27 in 19d71b7de0 outdated

27-    //! Feerate to use if overrideFeeRate is true
28-    CFeeRate nFeeRate;
29-    //! Override the default confirmation target, 0 = use default
30-    int nConfirmTarget;
31+    //! Override the default payTxFee if set, 0 = use smart fee estimation
32+    boost::optional<CFeeRate> m_feerate;

TheBlueMatt commented at 10:18 pm on July 9, 2017:

Can you write out more docs here? “m_feerate unset uses global paytxfee (if set) otherwise uses smart fee estimation. m_feerate set to 0 ignores global paytxfee and uses smart fee estimation. m_feerate set skips fee estimation but applies minTxFee and maxTxFee” “fOverrideFeeRate overrides all other feerate options (global paytxfee, smart fee estimation, txconfirmtarget, min+maxTxFee, etc) and uses m_feerate”

morcos commented at 1:05 am on July 10, 2017:

(see above, will document when we have a final logic)

in src/qt/walletmodel.cpp:674 in 19d71b7de0 outdated

668@@ -667,8 +669,9 @@ bool WalletModel::bumpFee(uint256 hash)
669 {
670     std::unique_ptr<CFeeBumper> feeBump;
671     {
672+        CCoinControl no_coin_control;
673         LOCK2(cs_main, wallet->cs_wallet);
674-        feeBump.reset(new CFeeBumper(wallet, hash, nTxConfirmTarget, false, 0, true));
675+        feeBump.reset(new CFeeBumper(wallet, hash, no_coin_control, 0));

TheBlueMatt commented at 10:25 pm on July 9, 2017:

This changes the newTxReplaceable option to fWalletRbf instead of true. Maybe leave it the way it was, or was this on purpose?

morcos commented at 0:58 am on July 10, 2017:

Good catch. Will fix. I think I just assumed how the code worked and didn’t even look at what I was replacing.

TheBlueMatt commented at 11:09 pm on July 9, 2017: member

Looks good, definitely want this to go in with #10589 (I’m super not a fan of CalculateEstimateType as an GetMinimumFee helper, but its easy to get rid of after this PR, less so after 10589).

morcos force-pushed on Jul 10, 2017

morcos commented at 4:45 pm on July 10, 2017: member

Rebased against the updated #10589 and fixed bug as well as improved parameter precedence logic for Coin Control.

in src/wallet/wallet.cpp:2570 in f07480b200 outdated

2566@@ -2567,7 +2567,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2567         LOCK2(cs_main, cs_wallet);
2568         {
2569             std::vector<COutput> vAvailableCoins;
2570-            AvailableCoins(vAvailableCoins, true, coinControl);
2571+            AvailableCoins(vAvailableCoins, true, &coin_control);

ryanofsky commented at 10:58 pm on July 10, 2017:

In commit “Make CoinControl a required argument to CreateTransaction”

Would be nice to make coin control a required argument to AvailableCoins as well. (To get rid of more multiply-defined default values.)

morcos commented at 0:32 am on July 11, 2017:

I agree, but decided it would make this PR a bit too big.

~~Which multiply defined values are you referring to though?~~ (edit: i think you just mean too many arguments with default values defined)

ryanofsky commented at 2:20 pm on July 11, 2017:

Which multiply defined values are you referring to though?

I was just referring to the fact that functions accepting null coincontrol pointers have to figure out their own default values instead of using the ones in CoinControl::SetNull.

in src/wallet/wallet.cpp:2620 in f07480b200 outdated

2616@@ -2617,7 +2617,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2617                 // Choose coins to use
2618                 CAmount nValueIn = 0;
2619                 setCoins.clear();
2620-                if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl))
2621+                if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, &coin_control))

ryanofsky commented at 10:58 pm on July 10, 2017:

In commit “Make CoinControl a required argument to CreateTransaction”

Would be nice if to make coin control a required argument to SelectCoins as well.

in src/wallet/coincontrol.h:46 in 9e70005f63 outdated

42@@ -41,9 +43,9 @@ class CCoinControl
43         fAllowOtherInputs = false;
44         fAllowWatchOnly = false;
45         setSelected.clear();
46-        nFeeRate = CFeeRate(0);
47+        m_feerate = boost::none;

ryanofsky commented at 11:11 pm on July 10, 2017:

In commit “Refactor to use CoinControl in GetMinimumFee and FeeBumper”

Maybe replace = boost::none with .reset() here and other places to remove boost reference and make it a little easier to port to std::optional in the future.

morcos commented at 0:32 am on July 11, 2017:

will do

morcos commented at 2:46 pm on July 11, 2017:

Actually turns out .reset() is deprecated so left it as is.

ryanofsky commented at 3:23 pm on July 11, 2017:

Actually turns out .reset() is deprecated so left it as is.

.reset is part of c++17 (http://en.cppreference.com/w/cpp/utility/optional/reset) so I still think it would be better to switch to this. (I don’t think we’d be using boost::optional at all now if we weren’t planning to switch to std::optional in the future.)

in src/wallet/coincontrol.h:25 in 9e70005f63 outdated

20@@ -19,12 +21,12 @@ class CCoinControl
21     bool fAllowOtherInputs;
22     //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria
23     bool fAllowWatchOnly;
24-    //! Override estimated feerate
25+    //! Override automatic min/max checks on fee, m_feerate must be set if true
26     bool fOverrideFeeRate;

ryanofsky commented at 11:17 pm on July 10, 2017:

In commit “Refactor to use CoinControl in GetMinimumFee and FeeBumper”

You should definitely get rid of the fOverrideFeeRate member now that m_feerate is optional.

morcos commented at 0:28 am on July 11, 2017:

Well.. almost fOverrideFeeRate is used from fundrawtransaction and it seems to me that it might make sense that you don’t want your specified fee rate subject to clamping by various min fees or the maxTxFee when you are using it from there but you do generally.

In any case it would be a change of functionality to change that, so we should do it separately.

ryanofsky commented at 6:31 pm on July 11, 2017:

fOverrideFeeRate is used from fundrawtransaction

Sorry, misread fOverrideFeeRate comment. And this is now pretty clear with fOverrideFeeRate handled in GetMinimumFee.

morcos force-pushed on Jul 11, 2017

morcos commented at 2:47 pm on July 11, 2017: member

Rebased cleanly so only new commits are left after #10589 was merged

in src/wallet/wallet.cpp:2723 in d72b3a52ee outdated

2728-                bool conservative_estimate = CalculateEstimateType(coinControl ? coinControl->m_fee_mode : FeeEstimateMode::UNSET, rbf);
2729-
2730-                CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc, false /* ignoreGlobalPayTxFee */, conservative_estimate);
2731-                if (coinControl && coinControl->fOverrideFeeRate)
2732-                    nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes);
2733+                // Allow to override automatic fee calculation (min/max checks) over coin control instance

TheBlueMatt commented at 3:31 pm on July 11, 2017:

It would be nice to move this check into GetMinimumFee so that everything is ompletely in the same place.

morcos commented at 5:44 pm on July 11, 2017:

done

morcos force-pushed on Jul 11, 2017

morcos commented at 5:49 pm on July 11, 2017: member

Rebased to accomodate #10712 and moved fOverrideFeeRate inside GetMinimumFee

in src/wallet/rpcwallet.cpp:2922 in fb5a21293b outdated

2923-            // instead of the confirmation target.
2924-            ignoreGlobalPayTxFee = true;
2925-            newConfirmTarget = options["confTarget"].get_int();
2926-            if (newConfirmTarget <= 0) { // upper-bound will be checked by estimatefee/smartfee
2927+            int target = options["confTarget"].get_int();
2928+            if (target <= 0) { // FIXME: Check upper bound too

ryanofsky commented at 6:28 pm on July 11, 2017:

In commit “Refactor to use CoinControl in GetMinimumFee and FeeBumper”

Why this fixme? Is the previous comment about upper bound not actually true?

morcos commented at 3:55 pm on July 12, 2017:

Correct, that comment about upper-bound is not true, I don’t know why it was put there. At the time I wrote these PR’s I had a lot of important stuff I wanted to make sure got in, so I didn’t want to fix every last little thing, but I think I could easily add a commit now that checks the bounds everywhere necessary.

in src/qt/coincontroldialog.cpp:590 in fb5a21293b outdated

586@@ -587,7 +587,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
587     if (payTxFee.GetFeePerK() > 0)
588         dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), payTxFee.GetFeePerK()) / 1000;
589     else {
590-        dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(coinControl->nConfirmTarget, NULL, ::mempool, conservative_estimate).GetFeePerK()) / 1000;
591+        dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(*coinControl->m_confirm_target, NULL, ::mempool, conservative_estimate).GetFeePerK()) / 1000;

ryanofsky commented at 6:49 pm on July 11, 2017:

In commit “Refactor to use CoinControl in GetMinimumFee and FeeBumper”

Maybe assert(coinControl->m_confirm_target) somewhere in this function. The event handling code which sets this seems a little precarious, so it’d be good to know if coinControl hasn’t been initialized correctly.

morcos commented at 4:05 pm on July 12, 2017:

seems reasonable

morcos commented at 6:56 pm on July 12, 2017:

this one went away, but addressed a similar one in qt/sendcoinsdialog.cpp

in src/qt/sendcoinsdialog.cpp:675 in 99e958f3f0 outdated

653-    {
654-        ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(),
655-                                                                std::max(CWallet::fallbackFee.GetFeePerK(), CWallet::GetRequiredFee(1000))) + "/kB");
656+    ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), feeRate.GetFeePerK()) + "/kB");
657+
658+    if (feeCalc.reason == FeeReason::FALLBACK) {

ryanofsky commented at 8:31 pm on July 11, 2017:

In commit “Make QT fee displays use GetMinimumFee instead of estimateSmartFee”:

Is this sufficient to know fee estimation succeeded? For example would it be possible for estimate to fail, and then fallbackFee to be less than RequiredFee, so reason would be REQUIRED not FALLBACK?

Also can you expand commit description to say a little bit about why it’s better to display minimum fee instead of estimated fee directly here. It seems perfectly reasonable, I’d just like to understand why the min fee wasn’t being being used all along, and whether one value is clearly more useful than the other or if this is more intended to be a minor tweak that simplifies the code.

morcos commented at 4:04 pm on July 12, 2017:

I think the behavior in rare edge cases is maybe not precisely correct, but there is likely no harm in it. The only thing you are missing is a warning that fee estimation is not up to date yet, and before this PR that could already happen if there was a mempool min fee (in which case you wouldn’t even get maxed with the fallback fee). After this PR, you only miss the warning if your fallback fee is less than mempool min fee or minRelayTxFee or minTxFee. The first case is a strict improvement over prior behavior. The second two cases can only result from very poor configuration of command line options. In short, I think it could be improved to report even more precise information to the user but it’s outside the scope of what I wanted to do here.

I can expand commit description, but the idea is the displays should correspond to the fees that your actually about to put on the transaction if you click send. I think it’s essentially a bug that it didn’t do that before.

ryanofsky commented at 5:52 pm on July 12, 2017:

I think the behavior in rare edge cases is maybe not precisely correct, but there is likely no harm in it.

I see. This is less significant than I thought because the fee is shown either way, just the labeling is different.

I can expand commit description, but the idea is the displays should correspond to the fees that your actually about to put on the transaction if you click send. I think it’s essentially a bug that it didn’t do that before.

Thanks, I didn’t know if the motivation was to simplify code or if min fee was actually preferable to display. Fact that min fee is used to send the transaction definitely makes sense as a reason to display it.

ryanofsky commented at 8:41 pm on July 11, 2017: member

utACK fb98c64134a3334d4d892af5ff08bc4291ea7b82

TheBlueMatt commented at 1:29 am on July 12, 2017: member

utACK fb98c64134a3334d4d892af5ff08bc4291ea7b82

morcos force-pushed on Jul 12, 2017

morcos commented at 6:58 pm on July 12, 2017: member

Addressed @ryanofsky feedback and added a commit to do proper bounds checking. boost::none’s were just all removed in one commit for simplicity.

(feelogic.ver2) -> c494da9 (feelogic.ver2.squash)

ryanofsky commented at 7:17 pm on July 12, 2017: member

utACK c494da90aa73051f3a21630f40ea84220e6e8bbb. Changes were adding qt coincontrol asserts, removing uses of boost::none, and adding a new commit with more conf_target checking.

in src/wallet/wallet.cpp:2943 in c494da90aa outdated

2950+    if (coin_control.m_feerate) {
2951+        fee_needed = coin_control.m_feerate->GetFee(nTxBytes);
2952         if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE;
2953+        // Allow to override automatic min/max check over coin control instance
2954+        if (coin_control.fOverrideFeeRate) return fee_needed;
2955+    } else {

jnewbery commented at 5:44 pm on July 13, 2017:

nit: I think this is clearer as:

0if {
1    ...
2} else if {
3    ...
4} else {
5    ...
6}

rather than:

0if {
1    ...
2} else {
3    if {
4        ...
5    } else {
6        ...
7    }
8}

(to match the logical construction of the comment above)

ryanofsky commented at 7:09 pm on July 13, 2017:

I wondered same thing initially, but I think it was written this way to unify the logic behind cases #2 and #4 which are very similar.

in src/wallet/wallet.cpp:2472 in c494da90aa outdated

2468@@ -2469,7 +2469,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
2469 
2470     CReserveKey reservekey(this);
2471     CWalletTx wtx;
2472-    if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, false))
2473+    if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false))

jnewbery commented at 5:45 pm on July 13, 2017:

supernit: braces

in src/wallet/wallet.cpp:2578 in c494da90aa outdated

2576             CScript scriptChange;
2577 
2578             // coin control: send change to custom address
2579-            if (coinControl && !boost::get<CNoDestination>(&coinControl->destChange))
2580-                scriptChange = GetScriptForDestination(coinControl->destChange);
2581+            if (!boost::get<CNoDestination>(&coin_control.destChange))

jnewbery commented at 5:45 pm on July 13, 2017:

nit: braces

in src/wallet/wallet.cpp:2955 in c494da90aa outdated

2962+            if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true;
2963+            else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false;
2964+
2965+            fee_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate).GetFee(nTxBytes);
2966+            if (fee_needed == 0) {
2967+                // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee

jnewbery commented at 5:55 pm on July 13, 2017:

The ... unless comment doesn’t make sense now that you’ve removed the earlier use -txconfirmtarget to estimate... comment. Perhaps just remove the ... unless?

in src/rpc/mining.h:15 in c494da90aa outdated

11@@ -12,4 +12,7 @@
12 /** Generate blocks (mine) */
13 UniValue generateBlocks(std::shared_ptr<CReserveScript> coinbaseScript, int nGenerate, uint64_t nMaxTries, bool keepScript);
14 
15+/** Check bounds on a command line confirm target */

jnewbery commented at 6:57 pm on July 13, 2017:

Why is this in rpc/mining.cpp? All of the calls to this function are in wallet/rpcwallet.cpp. Can you just put the function there?

Also the comment is a bit misleading - this function checks confirm target in RPCs, not command line arguments.

morcos commented at 7:15 pm on July 13, 2017:

#10707 adds it to the RPC calls in rpc/mining.cpp and rpcwallet.cpp already includes rpc/mining.h

jnewbery commented at 6:59 pm on July 13, 2017: member

utACK c494da90aa73051f3a21630f40ea84220e6e8bbb . I’m not very familiar with the qt code, so I can’t give you much feedback there, but the changes all look sensible.

A few nits. All could be addressed after merge (or left entirely).

morcos commented at 9:13 pm on July 13, 2017: member

@jnewbery See if you like the reorganization of GetMinimumFee and if so I’ll squash both commits

jnewbery commented at 9:34 pm on July 13, 2017: member

See if you like the reorganization of GetMinimumFee

I wasn’t necessarily advocating for changing the order of the conditionals, just for flattening the nested ifs. I’m happy in either order.

Definitely like the extra comments referring to which of the 4 branches the blocks match up to.

The indentation on LL2942-2943 is incorrect. Fix that and I’m happy!

morcos force-pushed on Jul 14, 2017

morcos commented at 3:07 pm on July 14, 2017: member

addressed @jnewbery nits and slight refactor of logic in GetMinimumFee (same end result) (feelogic.ver4) -> 759db91 (feelogic.ver4.squash)

Make CoinControl a required argument to CreateTransaction ecd81dfa3c

Refactor to use CoinControl in GetMinimumFee and FeeBumper

Improve parameter precedence in coin_control

03ee701161

Use CoinControl to pass custom fee setting from QT.

This fixes buggy behavior where we were temporarily setting and unsetting the
global payTxFee when trying to send a transaction with a custom fee from the
GUI. The previous behavior was inconsistent depending on the order of using the
RPC call settxfee and clicking various radio buttons in the sendcoinsdialog.
The new behavior is that transactions sent with the GUI will always use either
the smartfee slider value or the custom fee set on the GUI and they will not
affect the global defaults which are only for RPC and initial GUI values.

1983ca6cb3

Make QT fee displays use GetMinimumFee instead of estimateSmartFee

Remove helper function (CalculateEstimateType) for determining whether
estimates should be conservative or not, now that this is only called
once from GetMinimumFee and incorporate the logic directly there.

2fffaa9738

Remove checking of mempool min fee from estimateSmartFee.

This check has been moved to the wallet logic GetMinimumFee. The rpc call to
estimatesmartfee will now no longer return a result maxed with the mempool min
fee, but automated fee calculations from the wallet will produce the same result
as before and coincontrol and sendcoins dialogs in the GUI will correctly
display the right prospective fee.

changes to policy/fees.cpp include a big whitespace indentation change.

fd29d3df29

Properly bound check conf_target in wallet RPC calls 11590d39b9

morcos force-pushed on Jul 15, 2017

morcos commented at 3:48 am on July 15, 2017: member

Unfortunately this required a bit of a rebase due to conflict with #10769 in qt/sendcoinsdialog.cpp Other files had clean rebase.

gmaxwell commented at 4:03 am on July 15, 2017: contributor

concept ACK

in src/rpc/mining.cpp:818 in fd29d3df29 outdated

814@@ -815,7 +815,6 @@ UniValue estimatesmartfee(const JSONRPCRequest& request)
815             "\n"
816             "A negative value is returned if not enough transactions and blocks\n"
817             "have been observed to make an estimate for any number of blocks.\n"
818-            "However it will not return a value below the mempool reject fee.\n"

sipa commented at 6:42 pm on July 15, 2017:

Perhaps worth pointing out that the result may not be sufficient to be acceptable for the mempool?

morcos commented at 6:56 pm on July 15, 2017:

I think we should at least flag the change in the release notes but I don’t think that’s necessarily something users need to worry about. In the event that the tx isn’t accepted to the mempool it’s still likely’ish to be confirmed within the target by being resubmitted after mempool min fee decays. Which makes me wonder if we should even be worrying about mempool min fee in the GetMinimumFee logic but enough changes for now.

in src/wallet/wallet.cpp:2941 in 03ee701161 outdated

2945+        fee_needed = coin_control.m_feerate->GetFee(nTxBytes);
2946+        if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE;
2947+        // Allow to override automatic min/max check over coin control instance
2948+        if (coin_control.fOverrideFeeRate) return fee_needed;
2949+    }
2950+    else if (!coin_control.m_confirm_target && ::payTxFee != CFeeRate(0)) { // 3. TODO: remove magic value of 0 for global payTxFee

sipa commented at 6:44 pm on July 15, 2017:

Nit: else on the same line

sipa commented at 6:46 pm on July 15, 2017: member

Concept ACK. I very much like making everything uniformly use a coin control object for guiding estimation, and making the GUI’s coin control independent from global settings.

Code looks good, but I haven’t had time to review all logic changes.

TheBlueMatt commented at 10:18 pm on July 16, 2017: member

re-utACK 11590d39b9888403ead8354302e308eca139ba17

gmaxwell approved

gmaxwell commented at 6:31 am on July 17, 2017: contributor

ACK

in src/qt/sendcoinsdialog.cpp:270 in 11590d39b9

267@@ -274,14 +268,10 @@ void SendCoinsDialog::on_sendButton_clicked()
268     CCoinControl ctrl;
269     if (model->getOptionsModel()->getCoinControlFeatures())
270         ctrl = *CoinControlDialog::coinControl;

laanwj commented at 7:24 am on July 17, 2017:

Thanks. Definitely seems an improvement with regard to CCoinControl handling. Hopefully in a next PR we can finally get rid of this global object too.

laanwj commented at 7:25 am on July 17, 2017: member

utACK 11590d3

laanwj merged this on Jul 17, 2017

laanwj closed this on Jul 17, 2017

laanwj referenced this in commit 6859ad2936 on Jul 17, 2017

sipa referenced this in commit 75b5643c47 on Jul 17, 2017

laanwj referenced this in commit 8537187d42 on Jul 25, 2017

PastaPastaPasta referenced this in commit c849708996 on Sep 3, 2019

PastaPastaPasta referenced this in commit 93754f8d85 on Sep 4, 2019

PastaPastaPasta referenced this in commit ee4f4d343b on Sep 16, 2019

PastaPastaPasta referenced this in commit d04633d28c on Sep 18, 2019

barrystyle referenced this in commit 7013c8b341 on Jan 22, 2020

barrystyle referenced this in commit d7c88c0a92 on Jan 22, 2020

barrystyle referenced this in commit 0c317684a0 on Jan 22, 2020

jonatack referenced this in commit afe94710ef on Nov 12, 2020

jonatack referenced this in commit 05e82d86b0 on Nov 12, 2020

janus referenced this in commit 2a92a5fba8 on Dec 13, 2020

DrahtBot locked this on Sep 8, 2021

Improve wallet fee logic and fix GUI bugs #10706