Subtract fee from amount (rebase) #5831

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

Resolve conflicts w/ moved src/core.h
Resolve conflict in CoinControlDialog::updateLabels function
Port the RPC test to python

laanwj renamed this:
~~2015 02 cozz subtractfee~~
Subtract fee from amount (rebase)
on Feb 26, 2015

in src/qt/coincontroldialog.cpp:None 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

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.

laanwj added the label Wallet on Feb 27, 2015

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.

[Qt] Code-movement-only: Format confirmation message in sendcoinsdialog 90a43c1e93

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.

in src/rpcwallet.cpp:None 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?

laanwj force-pushed on Mar 5, 2015

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.

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.

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

jonasschnelli commented at 9:08 AM on March 6, 2015: contributor

Re-ACK (including @luke-jr commits)

cozz commented at 9:50 PM on March 7, 2015: contributor

help-description of sendmany is now wrong after switching from object to array

luke-jr commented at 9:51 PM on March 7, 2015: member

@cozz What's wrong about it? Looks fine to me?

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

laanwj commented at 11:38 AM on March 9, 2015: member

@luke-jr @cozz Added a commit that fixes the help message.

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

rpcwallet/sendmany: Just take an array of addresses to subtract fees from, rather than an Object with all values being identical 40a757331a

qa/rpc-tests/wallet: Tests for sendmany 1d9b378c30

laanwj force-pushed on Mar 13, 2015

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

jonasschnelli commented at 4:36 PM on March 13, 2015: contributor

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

bildschirmfoto 2015-03-13 um 17 35 32

laanwj commented at 11:23 AM on March 16, 2015: member

Thanks for testing @jonasschnelli

laanwj merged this on Mar 16, 2015

laanwj closed this on Mar 16, 2015

laanwj referenced this in commit df5c246ba3 on Mar 16, 2015

MarcoFalke locked this on Sep 8, 2021