Skip to content

Conversation

jonasschnelli
Copy link
Contributor

Fixes #7963

Currently fundrawtransaction always uses the default confirm target to estimate the fee (with a fallback to payTxFee).

This PR adds an optional feeRate parameter to fundrawtransaction that overrides the estimated minimum fee.

@@ -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);
Copy link
Member

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'.

Copy link
Contributor Author

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.

@jonasschnelli
Copy link
Contributor Author

Added a commit that add an explicit boolean for overriding the estimated fee rate (overrideFeeRate). This would allow to use 0 as fee rate (zero fee transaction) but would require 0 as minRelayTxFee.

@sipa
Copy link
Member

sipa commented May 17, 2016

utACK 04eaa90

@laanwj
Copy link
Member

laanwj commented Jun 3, 2016

utACK 04eaa90

@laanwj laanwj merged commit 04eaa90 into bitcoin:master Jun 3, 2016
laanwj added a commit that referenced this pull request Jun 3, 2016
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, )
Copy link
Member

@maflcko maflcko Jun 3, 2016

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)

@maflcko
Copy link
Member

maflcko commented Jun 3, 2016

Post merge utACK 04eaa90

codablock pushed a commit to codablock/dash that referenced this pull request Dec 22, 2017
04eaa90 Add more clear interface for CoinControl.h regarding individual feerate (Jonas Schnelli)
3b35e48 [RPC] add feerate option to fundrawtransaction (Jonas Schnelli)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
04eaa90 Add more clear interface for CoinControl.h regarding individual feerate (Jonas Schnelli)
3b35e48 [RPC] add feerate option to fundrawtransaction (Jonas Schnelli)
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Jun 28, 2020
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optional feerate option for fundrawtransaction
4 participants