fundrawtransaction
, this gives the user the option to specify whether new inputs should be added when bumping the fee. See #20935
rpc: add the add_inputs option to bumpfee/psbtbumpfee #21284
pull danben wants to merge 1 commits into bitcoin:master from danben:add-add-inputs-option-to-bumpfee changing 2 files +18 −2-
danben commented at 6:46 pm on February 23, 2021: contributorAs in
-
DrahtBot added the label RPC/REST/ZMQ on Feb 23, 2021
-
DrahtBot added the label Wallet on Feb 23, 2021
-
in src/wallet/rpcwallet.cpp:3447 in 40bde1a889 outdated
3443@@ -3444,6 +3444,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) 3444 "are replaceable).\n"}, 3445 {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" 3446 " \"" + FeeModes("\"\n\"") + "\""}, 3447+ {"add_inputs", RPCArg::Type::BOOL, /* default */ "true", "For a transaction with existing inputs, automatically include more if they are not enough."},
luke-jr commented at 3:36 am on February 24, 2021:Default should probably be “as needed”, and let “true” (perhaps not now, but someday) be used to force additional inputs?luke-jr changes_requestedghost commented at 8:16 pm on February 24, 2021: noneCompiled successfully. Tests failed.
Tried experimenting with
add_inputs
:- Transaction with 2 inputs selected manually and custom change address (A new receive address created and used as change):
FALSE
Error:
TRUE
- Transaction with 2 inputs selected manually and custom change address (A new change address created and used as change):
FALSE
No Error:
Not sure if this option is really solving the issue or will help users until we fix other related issues.
Added the add_inputs option to the bumpfee RPC. Behavior is identical to that of the same option in the fundrawtransaction RPC. 115b461b63danben force-pushed on Feb 25, 2021danben commented at 6:19 pm on February 25, 2021: contributor@luke-jr I made the suggested change but am a little worried that it won’t be clear to users what value to use if they don’t want new inputs to be added. @prayank23 I will look into what causes a change address to behave differently and possibly propose something. I agree that the current change will make things worse if the behavior is inconsistent.ghost commented at 6:57 pm on February 25, 2021: nonedanben commented at 2:35 am on February 27, 2021: contributor@prayank23 How did you create the change address in your second test case (i.e. the one that behaved wrongly)? Did you usegetrawchangeaddress
or something else?in src/wallet/rpcwallet.cpp:3447 in 115b461b63
3443@@ -3444,6 +3444,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) 3444 "are replaceable).\n"}, 3445 {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" 3446 " \"" + FeeModes("\"\n\"") + "\""}, 3447+ {"add_inputs", RPCArg::Type::STR, /* default */ "as_needed", "For a transaction with existing inputs, automatically include more if they are not enough."},
MarcoFalke commented at 1:07 pm on March 29, 2021:Wouldn’t this be better as a plain bool?
danben commented at 1:09 pm on April 1, 2021:@MarcoFalke That is what I had originally, but changed it to a string as per @luke-jr’s suggestion from 2/23 (see above)
MarcoFalke commented at 1:57 pm on April 1, 2021:what is the use case to force adding inputs if none are needed?
luke-jr commented at 2:12 am on June 23, 2021:I think you misunderstood me. I meant the documentation should be accurate, not that “as_needed” should be a value. Documenting it as “true” suggested that inputs would be added even if not needed.
If the user wants “as needed”, they just would omit “add_inputs” (or specify it as null). But I suppose defaults should be possible to specify explicitly, too, so not sure how to handle that. :)
DrahtBot commented at 1:57 pm on March 29, 2021: contributorThe following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #21544 (rpc: Missing doc updates for bumpfee psbt update by MarcoFalke)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot added the label Needs rebase on Apr 1, 2021DrahtBot commented at 9:05 am on April 1, 2021: contributor🐙 This pull request conflicts with the target branch and needs rebase.
Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.
DrahtBot commented at 12:28 pm on December 22, 2021: contributor- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.
fanquake added the label Up for grabs on Apr 26, 2022fanquake closed this on Apr 26, 2022
fanquake removed the label Up for grabs on Aug 4, 2022fanquake removed the label Needs rebase on Aug 4, 2022bitcoin locked this on Aug 4, 2023
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 18:12 UTC
More mirrored repositories can be found on mirror.b10c.me