[Wallet] Improve minimum absolute fee GUI options #7096

1. 
   jonasschnelli commented at 1:21 PM on November 25, 2015: contributor

   Currently, settxfee RPC call is broken. It is supposed to take a feerate (fee per KB), instead it uses the given feerate as absolute fee. We have even started taking this misconduct into RPC tests (https://github.com/bitcoin/bitcoin/blob/0.11/qa/rpc-tests/wallet.py#L107). (fixed by #7103)

   This PR fixes the settxfee RPC call (by fixing the underlaying problem) and decouples the UI CoinControl function for absolute min. fees from the global wallet state. Absolute fees without actually controlling the amount and type/size of inputs makes no sense and therefore this PR "hides" the absolute minimum fee behind the coin control feature and will only be enabled if the user has manually selected inputs.

   Includes work from @sipa. Thanks!

   Supersedes: #6708 Fixes: #6479

2. jonasschnelli added the label GUI on Nov 25, 2015
3. jonasschnelli added the label Wallet on Nov 25, 2015
4. jonasschnelli added the label Priority Medium on Nov 25, 2015
5. jonasschnelli added this to the milestone 0.12.0 on Nov 25, 2015
6. 
   jonasschnelli commented at 1:25 PM on November 25, 2015: contributor

   Supersedes: #6708 Fixes: #6479

7. 
   laanwj commented at 1:52 PM on November 25, 2015: member

   Concept ACK. Haven't looked closely at the code yet.

   Currently, settxfee RPC call is broken.

   That's pretty serious. Just curious: when did this break, did this problem make it into any release?

8. 
   jonasschnelli commented at 1:56 PM on November 25, 2015: contributor

   That's pretty serious. Just curious: when did this break, did this problem make it into any release?

   I try to unroll the bug-history right now. But I'm pretty sure the 0.11er releases are affected because this RPC tests (which tests the buggy behavior) is from the 0.11er branch https://github.com/bitcoin/bitcoin/blob/v0.11.0/qa/rpc-tests/wallet.py#L107).

9. 
   jonasschnelli commented at 2:02 PM on November 25, 2015: contributor

   I think the problem source lays in #5200 ... especially this line was a dangerous change (https://github.com/bitcoin/bitcoin/pull/5200/files#diff-d7618bdc04db23aa74d6a5a4198c58fdR1635)

   During the subtractFeeFromAmount work we started to take the wrong behavior into RPC tests: #5831

   However: this PR needs a backport to 0.11.

10. 
    MarcoFalke commented at 11:58 AM on November 26, 2015: member

    Supersedes: #6708

    Let's not call it "supersedes". It's an alternative bug fix with a similar result. Someone needs to check if the GUI changes are ok this way.

11. 
    laanwj commented at 12:18 PM on November 26, 2015: member

    I think it would be best to split this into two pulls:

    • Fix settxfee RPC
    • Improve the GUI

    At least the former (which is straightforward to test) must be backported to 0.11.

12. 
    jonasschnelli commented at 12:24 PM on November 26, 2015: contributor

    A quick fix was already opened (https://github.com/bitcoin/bitcoin/pull/6649). This PR would fix the settxfee RPC issue. But, if one uses the PRC together with the GUI, it can result in a wrong behavior. If a user presses the radio button "Use a minimum absolute fee of", suddenly all RPC send commands use the former settxfee feerate as absolute fee.

    We could merge and backport 6649 and fix the GUI within this PR.

13. in qa/rpc-tests/wallet.py:None in a3a5a14b0c outdated

     235 | +        assert_equal(self.nodes[2].getbalance(), node_2_bal + Decimal('4')); #should not be
     236 |  
     237 |          #send a tx with value in a string (PR#6380 +)
     238 |          txId  = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), "2")
     239 |          txObj = self.nodes[0].gettransaction(txId)
     240 | -        assert_equal(txObj['amount'], Decimal('-2.00000000'))
    

    ---

    ---

    

    MarcoFalke commented at 1:26 PM on November 26, 2015:

    @jonasschnelli Needs rebase on fa506c0c9b3928843704c666909c0b0c5af2f9a0 (#7103)

14. 
    MarcoFalke commented at 11:25 AM on November 30, 2015: member

    @jonasschnelli Mind to address the rebase "nit"?

15. Move fPayAtLeastCustomFee function to CC ecc7c82361
16. [Qt] use ASYMP_UTF8 (≈) whenever we show a fee that is not absolute 80462dda0a
17. [Qt] make use of the nMinimumTotalFee (absolute) in coincontrols fee calculation 31b508a18b
18. jonasschnelli force-pushed on Nov 30, 2015
19. [Qt] improve minimum absolute fee option

    - Only display the minimum absolute fee control if CoinControl is enabled

    ff723da6f6
20. jonasschnelli force-pushed on Nov 30, 2015
21. jonasschnelli renamed this:
    [Wallet] fix settxfee and improve minimum absolute fee GUI options
    [Wallet] Improve minimum absolute fee GUI options
    on Nov 30, 2015
22. 
    jonasschnelli commented at 1:21 PM on November 30, 2015: contributor

    Rebased and – after #7103 – it's now GUI only.

23. jonasschnelli removed the label Priority Medium on Nov 30, 2015
24. jonasschnelli removed the label Wallet on Nov 30, 2015
25. 
    btcdrak commented at 4:43 PM on November 30, 2015: contributor

    ACK

26. laanwj merged this on Dec 1, 2015
27. laanwj closed this on Dec 1, 2015

    ---

28. laanwj referenced this in commit 9490bd71bd on Dec 1, 2015
29. MarcoFalke locked this on Sep 8, 2021

Contributors

jonasschnellilaanwjMarcoFalkebtcdrak

Milestone
0.12.0