-
Notifications
You must be signed in to change notification settings - Fork 37.7k
[RPC] add feerate option to fundrawtransaction #7967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -2242,6 +2244,8 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt | |||
if (coinControl && nFeeNeeded > 0 && coinControl->nMinimumTotalFee > nFeeNeeded) { | |||
nFeeNeeded = coinControl->nMinimumTotalFee; | |||
} | |||
if (coinControl && coinControl->nFeeRate > CFeeRate(0)) | |||
nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK.
Though should we be using '0' as a special marker value? What if you want to override it to 0? I think I'd prefer an explicit boolean 'overrideFeeRate'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We could use an extra boolean (like overrideFeeRate
). My thoughts where that a CFeeRate(0)
is not practical but I agree, it would technically be possible to use a zero feerate. I'll add the bool.
Added a commit that add an explicit boolean for overriding the estimated fee rate ( |
utACK 04eaa90 |
utACK 04eaa90 |
04eaa90 Add more clear interface for CoinControl.h regarding individual feerate (Jonas Schnelli) 3b35e48 [RPC] add feerate option to fundrawtransaction (Jonas Schnelli)
inputs = [] | ||
outputs = {self.nodes[2].getnewaddress() : 1} | ||
rawtx = self.nodes[3].createrawtransaction(inputs, outputs) | ||
result = self.nodes[3].fundrawtransaction(rawtx, ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: empty arg.
Nit: Missing comment mentioning that min_relay_tx_fee=1000
is used.
(Comments help to identify the issue when something breaks)
Post merge utACK 04eaa90 |
04eaa90 Add more clear interface for CoinControl.h regarding individual feerate (Jonas Schnelli) 3b35e48 [RPC] add feerate option to fundrawtransaction (Jonas Schnelli)
04eaa90 Add more clear interface for CoinControl.h regarding individual feerate (Jonas Schnelli) 3b35e48 [RPC] add feerate option to fundrawtransaction (Jonas Schnelli)
fc81158 [QA] Add test_change_position case to rpc_fundrawtransaction.py (random-zebra) dd35760 [QA] Add test_option_feerate to rpc_fundrawtransaction functional test (random-zebra) 5bca4f4 Add more clear interface for CoinControl.h regarding individual feerate (random-zebra) 169bc3b [RPC] add feerate option to fundrawtransaction (random-zebra) 87dbdf8 [QA] Test new options in rpc_fundrawtransaction functional test (random-zebra) bc9dc67 Add lockUnspents option to fundrawtransaction (random-zebra) a3ac191 Add change options to fundrawtransaction (random-zebra) 0c1f7ba Add strict flag to RPCTypeCheckObj (random-zebra) d655b42 Use CCoinControl selection in CWallet::FundTransaction (random-zebra) 76c8d54 [QA] Test watchonly addrs in fundrawtransaction tests (random-zebra) 134c5d2 Implement watchonly support in fundrawtransaction (random-zebra) 1b153e5 Update importaddress help to push its use to script-only (random-zebra) 7b4eb6d Add importpubkey method to import a watch-only pubkey (random-zebra) 816dabb Add p2sh option to importaddress to import redeemScripts (random-zebra) 60a20a4 Split up importaddress into helper functions (random-zebra) cbffa80 Add logic to track pubkeys as watch-only, not just scripts (random-zebra) 12b38b0 Add have-pubkey distinction to ISMINE flags (random-zebra) fab6556 Exempt unspendable transaction outputs from dust checks (random-zebra) ab407ff [Tests] Fix and enable fundrawtransaction functional tests (random-zebra) bc44ba0 [wallet] allow transaction without change if keypool is empty (random-zebra) a2f8071 [wallet] CreateTransaction: simplify change address check (random-zebra) 761e60e Add fundrawtransaction RPC method (random-zebra) ccb18dd Add FundTransaction method to wallet (random-zebra) 692b827 Add DummySignatureCreator which just creates zeroed sigs (random-zebra) Pull request description: based on top of - [x] #1662 This introduces a new wallet function, `CWallet::FundTransaction()` (and exposes it via RPC with `fundrawtransaction`), to fill a tx containing only vouts (or not enough vins to cover the vouts) with unspent coins from the wallet. `fundrawtransaction` will not modify existing inputs, and will add one change output (if needed) to the outputs. It will not sign the inputs (so can include also watch-only or multi-sig inputs, if enabled). backported from: - bitcoin#6088 - bitcoin#17219 [`*`] - bitcoin#6417 - bitcoin#6444 - bitcoin#6415 - bitcoin#6828 - bitcoin#7296 (only bebe58b) - bitcoin#7506 - bitcoin#7518 - bitcoin#7967 adapting the tests for the (more recent) framework. [`*`] Note: this has been included to be able to call `fundrawtransaction` without the need for an unencrypted wallet (for the change address key) ACKs for top commit: furszy: re ACK fc81158 . Fuzzbawls: ACK fc81158 Tree-SHA512: 10235ce6e672a1cfd4ae2cad9312864c82971f6a4aa1a8ed9489d85156f5c4126c293180a7f1b86b7c65d07caab484e9a6d7a87ebf032bee55adb98d3e08e7b9
Fixes #7963
Currently
fundrawtransaction
always uses the default confirm target to estimate the fee (with a fallback topayTxFee
).This PR adds an optional
feeRate
parameter tofundrawtransaction
that overrides the estimated minimum fee.