[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. jonasschnelli cross-referenced this on Nov 25, 2015 from issue [wallet] Default fPayAtLeastCustomFee to false by MarcoFalke
8. 
   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?

9. 
   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).

10. 
    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.

11. 
    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.

12. 
    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.

13. 
    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.

14. MarcoFalke cross-referenced this on Nov 26, 2015 from issue [wallet, rpc tests] Fix settxfee, paytxfee by MarcoFalke
15. 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)

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

    @jonasschnelli Mind to address the rebase "nit"?

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

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

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

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

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

    ACK

28. laanwj merged this on Dec 1, 2015
29. laanwj closed this on Dec 1, 2015

    ---

30. laanwj referenced this in commit 9490bd71bd on Dec 1, 2015
31. MarcoFalke cross-referenced this on Mar 8, 2016 from issue paytxfee behavior changed without warning by mikegogulski
32. str4d cross-referenced this on Mar 29, 2017 from issue Bitcoin 0.12 wallet PRs by str4d
33. dagurval cross-referenced this on Aug 21, 2017 from issue [wallet, rpc tests] Fix settxfee, paytxfee by dagurval
34. dagurval cross-referenced this on Aug 21, 2017 from issue [wallet] remove minimum total fee option by dagurval
35. bitcoin locked this on Sep 8, 2021

Contributors

jonasschnellilaanwjMarcoFalkebtcdrak

Milestone
0.12.0