rpc: add the add_inputs option to bumpfee/psbtbumpfee#21284
rpc: add the add_inputs option to bumpfee/psbtbumpfee#21284danben wants to merge 1 commit intobitcoin:masterfrom
Conversation
|
Compiled successfully. Tests failed. Tried experimenting with
FALSE bumpfee fe60cda11767cd6c6c46a173a3ffa9cf3f71bc5f7af774ab641f6ede92fcd46a "{\"add_inputs\": false}"
Error: Unable to create transaction. Insufficient funds (code -4) TRUE bumpfee fe60cda11767cd6c6c46a173a3ffa9cf3f71bc5f7af774ab641f6ede92fcd46a "{\"add_inputs\": true}"
{
"txid": "2915edc7e7b8b375dc37a701146025b547903014e4a9652ba4dcb8cf77718ff8",
"origfee": 0.00000208,
"fee": 0.00005502,
"errors": [
]
}
FALSE bumpfee 86d1a738d652d4511810cf9c820b4bb294edcaff845ab9559ce4c7c33543662f "{\"add_inputs\": false}"
No Error: {
"txid": "a1a564ae1983e1a87e731c4b9b205f3e8edbd724bb03d4474090f32ccd8f55a0",
"origfee": 0.00000208,
"fee": 0.00001248,
"errors": [
]
}
Not sure if this option is really solving the issue or will help users until we fix other related issues. |
… to that of the same option in the fundrawtransaction RPC.
danben
force-pushed
the
add-add-inputs-option-to-bumpfee
branch
from
40bde1a to
115b461
Compare
|
@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. |
|
@danben Check #20795, bitcoin-core/gui#186 and #20598 |
|
@prayank23 How did you create the change address in your second test case (i.e. the one that behaved wrongly)? Did you use |
|
@danben I used Problem is with 2 things:
|
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
| "are replaceable).\n"}, | ||
| {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" | ||
| " \"" + FeeModes("\"\n\"") + "\""}, | ||
| {"add_inputs", RPCArg::Type::STR, /* default */ "as_needed", "For a transaction with existing inputs, automatically include more if they are not enough."}, |
There was a problem hiding this comment.
Wouldn't this be better as a plain bool?
There was a problem hiding this comment.
@MarcoFalke That is what I had originally, but changed it to a string as per @luke-jr's suggestion from 2/23 (see above)
There was a problem hiding this comment.
what is the use case to force adding inputs if none are needed?
There was a problem hiding this comment.
I will defer to @luke-jr on that; I am not sure
There was a problem hiding this comment.
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. :)
|
🐙 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". |
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
ghost
mentioned this pull request
5 participants
As in
fundrawtransaction, this gives the user the option to specify whether new inputs should be added when bumping the fee. See #20935