Add multiple options to fundrawtransaction#7518
Add multiple options to fundrawtransaction#7518promag wants to merge 3 commits intobitcoin:masterfrom
Conversation
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Mixed feelings about how to do this since the second argument can be one of two types.
There was a problem hiding this comment.
IIRC you can just leave off the second type for RPCTypeCheck and it will skip it. Then do the check by hand.
|
You need to update the help. I suggest replacing the Boolean with the Object entirely, and just supporting the boolean in code as a backward compatibility thing. |
30dfa20 to
97b60eb
Compare
|
@luke-jr done. |
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
The VOBJ check should be independent of RPCTypeCheck if you want to support boolean for backward compatibility.
There was a problem hiding this comment.
I see, so the help says it accepts a JSON object, but the implementation allows both?
97b60eb to
6235274
Compare
|
@luke-jr ok, added a test for the changeAddress option. |
6235274 to
7f5e281
Compare
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
This probably needs to be defaulting to CNoDestination()
7f5e281 to
9ef5802
Compare
|
@luke-jr done. |
8b3caf3 to
84b2bb9
Compare
84b2bb9 to
c0cfc61
Compare
|
Added changePosition option to specify the desired vout index of change address. |
qa/rpc-tests/fundrawtransaction.py
Outdated
There was a problem hiding this comment.
Can you leave one of them and mention that this is testing backward compatibility?
809044c to
709eda4
Compare
qa/rpc-tests/fundrawtransaction.py
Outdated
709eda4 to
49cea79
Compare
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
If we go for the backward compatibility way: I guess then this code part should be commented as "backward compatibility bool only fallback".
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
@jonasschnelli I think the best place to check if index is out of range is here.
Strict flag forces type check on all object keys.
bc4b4fb to
18be394
Compare
|
Tested ACK 18be394. This is very useful. |
|
utACK 18be394 |
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
…bt also lock manually selected coins 6d1f513 [rpc] fundrawtransaction, walletcreatefundedpsbt lock manually selected coins (Sjors Provoost) Pull request description: When using `fundrawtransaction` and `walletcreatefundedpsbt` with `lockUnspents`, it would only lock automatically selected coins, not manually selected coins. That doesn't make much sense to me if the goal is to prevent accidentally double-spending yourself before you broadcast a transaction. Note that when creating a transaction, manually selected coins are automatic "unlocked" (or more accurately: the lock is ignored). Earlier versions of this PR introduced an error when a locked coin is manually selected, but this idea was abandoned after some discussion. An application that uses this RPC should either rely on automatic coin selection (with `lockUnspents`) or handle lock concurrency itself with manual coin selection. In particular it needs to make sure to avoid/pause calls with automatic coin selection between calling `lockunspent` and the subsequent spending RPC. See bitcoin#7518 for historical background. ACKs for top commit: meshcollider: Code review ACK 6d1f513 fjahr: Code review ACK 6d1f513 Tree-SHA512: 8773c788d92f2656952e1beac147ba9956b8c5132d474e0880e4c89ff53642928b4cbfcd1cb3d17798b9284f02618a8830c93a9f7a4733e5bded96adff1d5d4d
This PR allows to specify an address to receive the change of the fund. Currently
fundrawtransactionaccepts a boolean for the second parameter. With this change the second parameter can be either a boolean or a JSON object with the following optional keys:includeWatching,changeAddressandchangePosition.