Subtract fee from amount (rebase) #5831

1. 
   laanwj commented at 1:15 pm on February 26, 2015: member

   Rebase #4331

   • Resolve conflicts w/ moved src/core.h
   • Resolve conflict in CoinControlDialog::updateLabels function
   • Port the RPC test to python
2. laanwj renamed this:
   2015 02 cozz subtractfee
   Subtract fee from amount (rebase)
   on Feb 26, 2015
3. in src/qt/coincontroldialog.cpp: in 8ee6ccd4f0 outdated

   544@@ -544,6 +545,11 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
   545         // Fee
   546         nPayFee = CWallet::GetMinimumFee(nBytes, nTxConfirmTarget, mempool);
   547 
   548+        // in the subtract fee from amount case, we can tell if zero change already and subtract the bytes, so that fee calculation afterwards is accurate
   

   ---

   ---

   

   cozz commented at 5:59 pm on February 26, 2015:

   this code-block needs to be moved one line above as the variable nBytes is used there

   ---

   

   laanwj commented at 5:03 pm on February 27, 2015:

   Thanks, good catch
4. 
   cozz commented at 6:01 pm on February 26, 2015: contributor

   nit: the second passage of the commit-message is now wrong, this was only true for the initial version of this pull-request, I’d say remove it, to avoid someone gets confused by it.
5. laanwj added the label Wallet on Feb 27, 2015
6. 
   jonasschnelli commented at 8:57 am on March 5, 2015: contributor

   needs rebase.

   GUI and RPC looks good. Nice @cozz! Thanks for the rebase and rework @laanwj.

   The new parameter for sendmany (json object) looks a bit as a overdose to me. A single boolean would maybe be sufficient.

   Tested ACK.

7. [Qt] Code-movement-only: Format confirmation message in sendcoinsdialog 90a43c1e93
8. 
   laanwj commented at 12:31 pm on March 5, 2015: member

   With regard to future extensibility, I like the idea of adding a JSON structure to sendmany. Should have been done that in the first place instead of the long-winded positional command lines for some methods.
9. in src/rpcwallet.cpp: in 8ee6ccd4f0 outdated

   803@@ -793,9 +804,9 @@ Value movecmd(const Array& params, bool fHelp)
   804 
   805 Value sendfrom(const Array& params, bool fHelp)
   806 {
   807-    if (fHelp || params.size() < 3 || params.size() > 6)
   808+    if (fHelp || params.size() < 3 || params.size() > 7)
   809         throw runtime_error(
   810-            "sendfrom \"fromaccount\" \"tobitcoinaddress\" amount ( minconf \"comment\" \"comment-to\" )\n"
   811+            "sendfrom \"fromaccount\" \"tobitcoinaddress\" amount ( minconf \"comment\" \"comment-to\" subtractfeefromamount )\n"
   

   ---

   ---

   

   luke-jr commented at 12:35 pm on March 5, 2015:

   Do we really want to extend deprecated methods?
10. laanwj force-pushed on Mar 5, 2015
11. laanwj force-pushed on Mar 5, 2015
12. laanwj force-pushed on Mar 5, 2015
13. 
    jonasschnelli commented at 12:38 pm on March 5, 2015: contributor

    Agreed. JSON structures as parameter (or as optional parameter) would be more convenient and extendable. sandmany and sendfrom are also marked for deprecation so i assume main focus should be GUI and sendtoaddress.
14. 
    laanwj commented at 12:40 pm on March 5, 2015: member

    eh, who is deprecating sendmany? If we were to keep one send* method, that would be the one. It is the most general send method.
15. 
    luke-jr commented at 12:41 pm on March 5, 2015: member

    @laanwj The JSON structure in the current PR only allows for a boolean for each recipient. Are you suggesting it be changed to a parameter object (eg, like getblocktemplate)? If so, I agree that seems like a better approach.

    As far as multiple recipients sharing the fee, it seems more logical to be using an Array than an Object with only values of “true”.

16. 
    jonasschnelli commented at 9:08 am on March 6, 2015: contributor

    Re-ACK (including @luke-jr commits)
17. 
    cozz commented at 9:50 pm on March 7, 2015: contributor

    help-description of sendmany is now wrong after switching from object to array
18. 
    luke-jr commented at 9:51 pm on March 7, 2015: member

    @cozz What’s wrong about it? Looks fine to me?
19. 
    cozz commented at 10:44 pm on March 7, 2015: contributor

    • “A json object with addresses and booleans” - its not an object but an array and it doesnt take booleans
    • “Default for each address is false” - this sentence should be removed
20. 
    laanwj commented at 11:38 am on March 9, 2015: member

    @luke-jr @cozz Added a commit that fixes the help message.
21. Subtract fee from amount

    Fixes #2724 and #1570.
    
    Adds the
    automatically-subtract-the-fee-from-the-amount-and-send-whats-left
    feature to the GUI and RPC (sendtoaddress,sendmany).

    292623adf5
22. rpcwallet/sendmany: Just take an array of addresses to subtract fees from, rather than an Object with all values being identical 40a757331a
23. qa/rpc-tests/wallet: Tests for sendmany 1d9b378c30
24. laanwj force-pushed on Mar 13, 2015
25. 
    laanwj commented at 10:17 am on March 13, 2015: member

    I’ve squashed the two squashme commits, as well as @luke-jr’s commit that reverts the changes to sendfrom (so that its API not touched at all).
26. 
    jonasschnelli commented at 4:36 pm on March 13, 2015: contributor

    Retested ACK. Binaries: https://builds.jonasschnelli.ch/pulls/5831/

    

27. 
    laanwj commented at 11:23 am on March 16, 2015: member

    Thanks for testing @jonasschnelli
28. laanwj merged this on Mar 16, 2015
29. laanwj closed this on Mar 16, 2015

    ---

30. laanwj referenced this in commit df5c246ba3 on Mar 16, 2015
31. MarcoFalke locked this on Sep 8, 2021

Contributors
laanwj cozz jonasschnelli luke-jr

Labels
Wallet