Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add subtractfeeamount parameter #1626

Merged
merged 6 commits into from Jun 14, 2017

Conversation

Projects
None yet
5 participants
Contributor

d2597b33-2ac1-4c27-af8e-5794285e8600 commented Jun 12, 2017 edited

Functionality was merged into Bitcoin Core in March '15 which I think is Bitcoin Core 0.10.1?

Here it is in the latest version: https://github.com/bitcoin/bitcoin/blob/e4918316d80f0541189c40039095c716a09d636e/src/wallet/rpcwallet.cpp#L403

Contributor

Mirobit commented Jun 12, 2017

LGTM

Contributor

jonasschnelli commented Jun 12, 2017

Ideally this change would also add the subtractfromfee parameter for sendmany and maybe fundrawtransaction.

Not related to this PR, but ideally someone should add a part for the name based RPC parameter option: bitcoin/bitcoin#8811

I'll add the parameter to sendmany and fundrawtransaction in a moment.

One small nit, otherwise tested ACK. I think we already have this documented for fundrawtransaction, but adding it for sendmany would be appreciated. Thanks @d2597b33-2ac1-4c27-af8e-5794285e8600 !

@@ -56,14 +56,24 @@ The `sendtoaddress` RPC {{summary_sendToAddress}}
{% enditemplate %}
+*Parameter #5---automatic fee subtraction
@harding

harding Jun 13, 2017

Contributor

Needs an asterisk and the end of this line to close the italics.

Contributor

harding commented Jun 13, 2017

@jonasschnelli you're the second person to suggest adding named parameters to me this week, so it's nearing the top of my todo list (I didn't realize support for that was added until the first person told me). Adding a note about it to the RPC intro section should be easy, but I think to fully add it to the docs we'll have to add all of the input parameter names to each RPC call, which will take a little bit more time. Thanks for the suggestion!

Fix markdown formatting
Add asterisk at the end of the line.

Updated review.

@@ -64,6 +64,11 @@ All existing inputs must have their previous output transaction be in the wallet
t: "numeric (bitcoins)"
p: "Optional<br>(0 or 1)"
d: "The specific feerate you are willing to pay(BTC per KB). If not set, the wallet determines the fee"
+
+- n: "→ <br>`subtractFeeFromOutputs`"
@harding

harding Jun 13, 2017

Contributor

This field is already documented (see lines 68-71 in the original or 73-76 in this version), and in this call the parameter is an array of integers not a bool.

@d2597b33-2ac1-4c27-af8e-5794285e8600

d2597b33-2ac1-4c27-af8e-5794285e8600 Jun 13, 2017

Contributor

Sorry, I don't know how I missed that.

@@ -55,6 +55,16 @@ The `sendmany` RPC {{summary_sendMany}}
{% enditemplate %}
+*Parameter #5---automatic fee subtraction
@harding

harding Jun 13, 2017

Contributor

Need an asterisk at the end of the line to close the italics.

+
+{% itemplate ntpd1 %}
+- n: "Subtract Fee From Amount"
+ t: "boolean"
@harding

harding Jun 13, 2017

Contributor

In the sendmany call, this is an array of strings (addresses) rather than a single bool for the whole transaction.

Fix subtractfeefrom parameter of sendmany
subtractfeefrom is an array of addresses, not a
bool.
Contributor

wbnns commented Jun 13, 2017

Unless others object, this will be merged on Wednesday, June 14th.

@wbnns wbnns self-assigned this Jun 13, 2017

Contributor

harding commented Jun 13, 2017

I sent @d2597b33-2ac1-4c27-af8e-5794285e8600 a quick PR to make the changes (and some related existing text) consistent with our normal style. Thanks again for catching this omission!

Merge pull request #1 from harding/edit-1626
Dev Docs: RPCs: edits for Subtract Fee From Amount params

@harding, I merged your PR into my branch. Hopefully I did it that correctly.

Thanks again :)

Contributor

harding commented Jun 13, 2017

ab16005 tested ACK. Thanks!

@wbnns wbnns merged commit d827ca0 into bitcoin-dot-org:master Jun 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@d2597b33-2ac1-4c27-af8e-5794285e8600 d2597b33-2ac1-4c27-af8e-5794285e8600 deleted the d2597b33-2ac1-4c27-af8e-5794285e8600:patch-1 branch Jun 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment