Skip to content

Commit

Permalink
Merge #16378: The ultimate send RPC
Browse files Browse the repository at this point in the history
92326d8 [rpc] add send method (Sjors Provoost)
2c2a144 [rpc] add snake case aliases for transaction methods (Sjors Provoost)
1bc8d0f [rpc] walletcreatefundedpsbt: allow inputs to be null (Sjors Provoost)

Pull request description:

  `walletcreatefundedpsbt` has some interesting features that `sendtoaddress` and `sendmany` don't have:
  * manual coin selection
  * outputting a PSBT (it was controversial to add this, see #18201)
  * create a transaction without adding to wallet (which leads to broadcasting, unless `-walletbroadcast=0`)

  At the same time `walletcreatefundedpsbt` can't broadcast a transaction, which is inconvenient for simple use cases.

  This PR introduces a new `send` RPC method which creates a PSBT, signs it if possible and adds it to the wallet by default. If it can't sign all inputs, it outputs a PSBT. If `add_to_wallet` is set to `false` it will return the transaction in both PSBT and hex format.

  Because it uses a PSBT internally, it will much easier to add hardware wallet support to this method (see #16546).

  For `bitcoin-cli` users, it tries to keep the simplest use case easy to use:

  ```sh
  bitcoin-cli -regtest send '{"ADDRESS": 0.1}' 1 sat/b
  ```

  This paves the way for deprecating `sendtoaddress` and `sendmany` though there's no rush. The only missing feature compared to these older methods is adding labels to a destination address.

  Depends on:
  - [x] #16377 (`[rpc] don't automatically append inputs in walletcreatefundedpsbt`)
  - [x] #11413 (`[wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option`)
  - [x] #18244 (`[rpc] have lockUnspents also lock manually selected coins`)

ACKs for top commit:
  meshcollider:
    Light re-utACK 92326d8
  achow101:
    ACK 92326d8 Reviewed code and test, ran tests.
  kallewoof:
    utACK 92326d8

Tree-SHA512: 7552ef1b193d4c06e381c44932fdb0d54f64383e4c7d6b988f49d059c7d4bba45ce6aa7813e03df86360ad9dad6f3010eb76ee7da480551742d5fd98c2251c0f
  • Loading branch information
meshcollider committed Sep 15, 2020
2 parents 06dbbe7 + 92326d8 commit ffaac6e
Show file tree
Hide file tree
Showing 8 changed files with 568 additions and 18 deletions.
5 changes: 5 additions & 0 deletions doc/release-notes-16378.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
RPC
---
- A new `send` RPC with similar syntax to `walletcreatefundedpsbt`, including
support for coin selection and a custom fee rate. Using the new `send` method
is encouraged: `sendmany` and `sendtoaddress` may be deprecated in a future release.
3 changes: 3 additions & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "gettxoutproof", 0, "txids" },
{ "lockunspent", 0, "unlock" },
{ "lockunspent", 1, "transactions" },
{ "send", 0, "outputs" },
{ "send", 1, "conf_target" },
{ "send", 3, "options" },
{ "importprivkey", 2, "rescan" },
{ "importaddress", 2, "rescan" },
{ "importaddress", 3, "p2sh" },
Expand Down
11 changes: 8 additions & 3 deletions src/rpc/rawtransaction_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@

CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf)
{
if (inputs_in.isNull() || outputs_in.isNull())
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, arguments 1 and 2 must be non-null");
if (outputs_in.isNull())
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null");

UniValue inputs;
if (inputs_in.isNull())
inputs = UniValue::VARR;
else
inputs = inputs_in.get_array();

UniValue inputs = inputs_in.get_array();
const bool outputs_is_obj = outputs_in.isObject();
UniValue outputs = outputs_is_obj ? outputs_in.get_obj() : outputs_in.get_array();

Expand Down
222 changes: 208 additions & 14 deletions src/wallet/rpcwallet.cpp

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test/functional/rpc_fundrawtransaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def test_invalid_change_address(self):
dec_tx = self.nodes[2].decoderawtransaction(rawtx)
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])

assert_raises_rpc_error(-5, "changeAddress must be a valid bitcoin address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'})
assert_raises_rpc_error(-5, "Change address must be a valid bitcoin address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'})

def test_valid_change_address(self):
self.log.info("Test fundrawtxn with a provided change address")
Expand Down
3 changes: 3 additions & 0 deletions test/functional/rpc_psbt.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ def run_test(self):
psbtx1 = self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():90}, 0, {"add_inputs": True})['psbt']
assert_equal(len(self.nodes[0].decodepsbt(psbtx1)['tx']['vin']), 2)

# Inputs argument can be null
self.nodes[0].walletcreatefundedpsbt(None, {self.nodes[2].getnewaddress():10})

# Node 1 should not be able to add anything to it but still return the psbtx same as before
psbtx = self.nodes[1].walletprocesspsbt(psbtx1)['psbt']
assert_equal(psbtx1, psbtx)
Expand Down
1 change: 1 addition & 0 deletions test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
'rpc_estimatefee.py',
'rpc_getblockstats.py',
'wallet_create_tx.py',
'wallet_send.py',
'p2p_fingerprint.py',
'feature_uacomment.py',
'wallet_coinbase_category.py',
Expand Down
339 changes: 339 additions & 0 deletions test/functional/wallet_send.py

Large diffs are not rendered by default.

0 comments on commit ffaac6e

Please sign in to comment.