Add createtransaction

promag commented at 1:00 pm on February 10, 2016: member

Syntax: createtransaction {"address":amount,...} ({ "option": value,...})

This call is very similar to sendmany but it also allows to:

use unspent transaction outputs of watch only addresses;
specify the change address;
optionally not commit the transaction;
optionally not sign the transaction;
include the scriptPubKey of transaction inputs.

Optional arguments are specified in a JSON object. The output similar is to decoderawtransaction.

This is very useful for merchants that doesn’t want a wallet with private keys but wishes to use the algorithm used in the unspent output selection and fee calculation. Then it can sign the inputs and send the raw transaction.

Example:

0bitcoin-cli -regtest createtransaction '{ "mpi744Kz6uDEDeTLDj8BwXbwfanjoVSPch": "20" }' '{ "allowWatchOnly": true, "changeAddress" : "mnBfCwG7Dc4HeJob681kRLSueUzQ7XMKQf", "commit": false, "includeSpentOutputs": true, "sign": false }'

TODO: optionally lock unspents when commit is false.

[WIP][RPC] Add createtransaction 32b4234c93

sipa commented at 1:02 pm on February 10, 2016: member

I believe the functionality you want is available through the new fundrawtransaction RPC already?

fixe commented at 1:31 pm on February 10, 2016: none

@sipa: fundrawtransaction does not allow to specify the change address. Also, having the scriptPubKey in the output avoids RPC calls later when signing. This call could eventually deprecate sendmany and sendtoaddress.

laanwj commented at 1:37 pm on February 10, 2016: member

If fundrawtransaction provides part of the functionality, what about extending it?

(fundtransaction is very new, so it’s kind confusing to add such as similar RPC call so soon after)

sipa commented at 1:39 pm on February 10, 2016: member

I agree that reporting the scriptPubKey of outputs being spent is very useful, but I’d prefer to see that functionality also available in fundrawtransaction.

laanwj added the label Wallet on Feb 10, 2016

promag commented at 3:07 pm on February 10, 2016: member

The call fundrawtransaction deals with raw transactions: accepts a raw transaction and returns the new raw transaction. It’s usage and parameters are clear.

The call createtransaction is more like swiss army knife to create transactions in a human readable way and follows a different argument syntax: options are in a JSON object (which is more convenient and scalable than ordered arguments).

The usage can go from a simple send value:

0bitcoin-cli -regtest createtransaction '{ "mpi744Kz6uDEDeTLDj8BwXbwfanjoVSPch": "20" }'

to prepare transactions for offline signing:

0bitcoin-cli -regtest createtransaction '{ "mpi744Kz6uDEDeTLDj8BwXbwfanjoVSPch": "20" }' '{ "includeSpentOutputs": true, "sign": false }'

At the end the same could be obtained with some RPC calls:

0$ bitcoin-cli -regtest createrawtransaction '[]' '{ "mpi744Kz6uDEDeTLDj8BwXbwfanjoVSPch": 20 }'
1$ bitcoin-cli -regtest fundrawtransaction <raw1> true
2$ bitcoin-cli -regtest decoderawtransaction <raw2>

With the exception of not having the list of spent scriptPubKeys (useful for signing).

For me stating that fundrawtransaction has part the of functionality so it should be extended does not apply. This is a more concise, obvious and library friend way to create transactions in a single RPC call.

At the moment I’m sticking with this unless the majority votes for extending fundrawtransaction.

laanwj commented at 3:22 pm on February 10, 2016: member

options are in a JSON object (which is more convenient and scalable than ordered arguments).

I completely agree on that.

However the general philosophy is to not have RPC calls that can be implemented in terms of each other.

If this turns out to be much more useful API than fundrawtransaction then we could deprecate that one. I just think it’s a waste, something that should ideally have been addressed as part of the review process for #6088 (which already superceeded PRs implementing similar commands, this didn’t come out of the blue).

Now, if we would merge this, we would have to carry two RPC calls that largely overlap in functionality for a while, which is a maintenance burden.

Edit:

For me stating that fundrawtransaction has part the of functionality so it should be extended does not apply. This is a more concise, obvious and library friend way to create transactions in a single RPC call.

Devil’s advocate: absolutely, but isn’t the “long way around” more flexible? Does combining the intermediate steps, all having a limited but well-defined tasks, allow doing combined operations that could never all be part of a single monolithic command?

promag commented at 5:49 pm on February 10, 2016: member

@laanwj the work in #6088 is very relevant, mainly the changes in CWallet::SelectCoins, CWallet::CreateTransaction and the ugly DummySignatureCreator.

There is no release with fundrawtransaction, so it is still possible to avoid that maintenance burden.

Edit:

Maybe I’m not seeing the use case, but whoever wants to call fundrawtransaction could call createtransaction, and this is far more intuitive than the first.

sipa commented at 5:52 pm on February 10, 2016: member

Fundrawtransaction is more flexible: it offers exact control over the outputs and using specific inputs in addition to the automatically selected ones.

in src/wallet/rpcwallet.cpp: in 32b4234c93

404+      return NullUniValue;
405+
406+  if (fHelp || params.size() < 1 || params.size() > 2)
407+      throw runtime_error(
408+          "createtransaction {\"address\":amount,...} ({ \"option\": value,...})\n"
409+          "\nCreate transaction. Amounts are double-precision floating point numbers."

sipa commented at 6:05 pm on February 10, 2016:

This is not correct. JSON does not have a concept of “double precision” or “floating poing”, only “number”. Furthermore, AmountFromValue accepts both numbers and numbers-encoded-as-strings since recently.

promag commented at 6:06 pm on February 10, 2016:

:+1: just copied that.. Will fix.

promag commented at 6:05 pm on February 10, 2016: member

@sipa this is WIP because one of the things I want to add is the option to lock the UTXOs and the option to select some inputs.

It is impossible to have concurrent fundrawtransaction calls. Becase the second call can select the same coins, and if they come from createrawtransaction then there is an issue.

promag commented at 6:07 pm on February 10, 2016: member

Fundrawtransaction is more flexible: it offers exact control over the outputs @sipa what do you mean?

sipa commented at 6:12 pm on February 10, 2016: member

Fundrawtransaction is more flexible: it offers exact control over the outputs

@sipa what do you mean?

You can pass in a raw transaction, which can have arbitrary outputs (including non-standard scriptPubKey and ones that don’t have a corresponding address), you have control over the order of the outputs, …

nunofgs commented at 6:19 pm on February 10, 2016: none

I too prefer the createtransaction call. It is closer to the actual concepts already introduced by the CWallet class:

0…
1bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true);
2bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey);
3…

I like the idea of “creating” a transaction and then, if everything is to my liking, “committing” it.

promag commented at 6:21 pm on February 10, 2016: member

@sipa I see, maybe then both make sense? Because, with that in mind, even sendtoaddress and sendmany could be removed/deprecated when fundrawtransaction was introduced..

sipa commented at 6:24 pm on February 10, 2016: member

@nunofgs It seems they both have advantages currently that the other doesn’t have:

Fundrawtransaction is more flexible as it integrates cleanly with raw transactions
Createtransaction reports which scriptPubKeys were spent. @nunofgs createrawtransaction + fundrawtransaction does exactly the same thing; except it breaks the creating up into two separate steps. @promag There’s no way we can ever remove sendtoaddress and sendmany. They’re likely the most commonly used RPCs that exist. @promag We’re not going to change the behaviour of an RPC call at 0.12.0rc4, but we can modify it in a backward-compatible way later.

laanwj commented at 6:48 pm on February 10, 2016: member

There is no release with fundrawtransaction, so it is still possible to avoid that maintenance burden.

It is much too late in the release cycle for 0.12 to change this around (in a backwards incompatible way).

promag commented at 10:58 pm on February 10, 2016: member

You can pass in a raw transaction, which can have arbitrary outputs (including non-standard scriptPubKey and ones that don’t have a corresponding address), you have control over the order of the outputs, … @sipa the way createrawtransaction receives the outputs is the same as createtransaction. Also the way the output argument is declared can cause problems for libraries regarding the key data, as stated in http://tools.ietf.org/html/rfc7159#section-4.

Anyway, I’ll add the functionality to fundrawtransaction by allowing the second argument to be either a boolean (includeWatching) or a JSON object ({ "option": value }) the same way it is implemented in createtransaction.

Edit:

I wonder if the consensus is to drop createtransaction..

promag commented at 11:17 pm on February 10, 2016: member

By the way, CWallet::CreateTransaction and createtransaction allows to specify who pays the fee. Makes sense to support this in fundrawtransaction by changing the outputs?

jonasschnelli commented at 4:14 pm on February 19, 2016: contributor

I also think we should add this to fundrawtransaction. I once also had appetite to add a swiss army like sendmanyraw function. But we should not forget that this is a RPC interface and should focus on a clear design and chains of commands (like createraw_, fundraw_, signraw*, etc.) makes the RPC interface flexible, combined superfuntions do more solve enduser functions which could be included in additional helper application like python scripts or bitcoin-cli like applications.

promag renamed this:
~~[WIP][RPC] Add createtransaction~~
Add createtransaction
on Feb 25, 2016

promag commented at 1:12 am on February 25, 2016: member

@laanwj missing rpc label on this PR.

promag commented at 1:13 am on February 25, 2016: member

Deprecated by low level calls createrawtransaction, fundrawtransaction and signrawtransaction.

promag closed this on Feb 25, 2016

DrahtBot locked this on Sep 8, 2021

Add createtransaction #7498