rpc: Note in fundrawtransaction doc, fee rate is for package #32607

pull benthecarman wants to merge 1 commits into bitcoin:master from benthecarman:fundawtx-pkg-doc changing 1 files +4 −1

benthecarman commented at 10:30 pm on May 23, 2025: contributor

Accidentally made some transactions with a much higher fee rate than I wanted because I did not know this would do it for the package rather than the individual tx.
DrahtBot commented at 10:30 pm on May 23, 2025: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32607.

Reviews

See the guideline for information on the review process.

Type Reviewers

ACK rkrux, i-am-yuvi

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:
- #32523 (wallet: Remove watchonly behavior and isminetypes by achow101)
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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:
- watch-only -> watch-only scripts [missing noun after adjective]
- , this is because -> . This is because [run-on sentence/comma splice]
drahtbot_id_0520
DrahtBot added the label RPC/REST/ZMQ on May 23, 2025

Type	Reviewers
ACK	rkrux, i-am-yuvi

in src/wallet/rpc/spend.cpp:764 in fc6d17f4d9 outdated

757@@ -758,6 +758,9 @@ RPCHelpMan fundrawtransaction()
758                 "in the wallet using importdescriptors (to calculate fees).\n"
759                 "You can see whether this is the case by checking the \"solvable\" field in the listunspent output.\n"
760                 "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n",
761+                "Note that if specifying an exact fee rate, the resulting transaction may have a higher fee rate\n"
762+                "if the transaction has unconfirmed inputs, this is because the wallet will attempt to make the\n"
763+                "entire package have the given fee rate, not the resulting transaction."
764                 {

davidgumberg commented at 10:34 pm on May 23, 2025:

0                "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n"
1                "Note that if specifying an exact fee rate, the resulting transaction may have a higher fee rate\n"
2                "if the transaction has unconfirmed inputs, this is because the wallet will attempt to make the\n"
3                "entire package have the given fee rate, not the resulting transaction.",
4                {

benthecarman commented at 10:35 pm on May 23, 2025:

woops

benthecarman force-pushed on May 23, 2025
DrahtBot added the label CI failed on May 23, 2025
DrahtBot commented at 10:35 pm on May 23, 2025: contributor
🚧 At least one of the CI tasks failed. Task macOS-cross, gui, no tests: https://github.com/bitcoin/bitcoin/runs/42814612722 LLM reason (✨ experimental): The CI failure is due to a build error in src/wallet/rpc/spend.cpp related to a missing closing curly brace.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
rpc: Note in fundrawtransaction doc, fee rate is for package fe0432b1c4
benthecarman force-pushed on May 23, 2025

in src/wallet/rpc/spend.cpp:760 in fe0432b1c4

756@@ -757,7 +757,10 @@ RPCHelpMan fundrawtransaction()
757                 "Note that all inputs selected must be of standard form and P2SH scripts must be\n"
758                 "in the wallet using importdescriptors (to calculate fees).\n"
759                 "You can see whether this is the case by checking the \"solvable\" field in the listunspent output.\n"
760-                "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n",
761+                "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n"

rkrux commented at 8:35 am on May 24, 2025:

I see it needs a full stop after watch-only.

rkrux commented at 8:39 am on May 24, 2025: contributor

ACK fe0432b1c4a10b74844c2dedefccfe340c0d3b10

This is a good note, helpful. I verified in the code that the outputs selected in the transaction have a corresponding ancestor_bump_fees param that is used in calculating the TotalBumpFees inside CreateTransactionInternal function.

https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/src/wallet/coinselection.h#L75-L76

https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/src/wallet/spend.cpp#L1251-L1257

Interesting that the feature_fee_estimation.py test fails in the CI coincidentally but seems unrelated to me as it passed in my system. Maybe a rerun/push would fix it?
maflcko commented at 10:54 am on May 24, 2025: member

Interesting that the feature_fee_estimation.py test fails in the CI coincidentally but seems unrelated to me as it passed in my system. Maybe a rerun/push would fix it?

See https://github.com/bitcoin/bitcoin/issues/32461
i-am-yuvi commented at 3:52 pm on May 24, 2025: contributor

ACK fe0432b1c4a10b74844c2dedefccfe340c0d3b10

This will be a helpful note for someone using the rpc.

Contributors
benthecarman DrahtBot davidgumberg rkrux maflcko i-am-yuvi

Labels
RPC/REST/ZMQ CI failed

rpc: Note in fundrawtransaction doc, fee rate is for package #32607

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)