diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -11,3 +11,4 @@ noincludeconf=1 includeconf=relative.conf as bitcoin.conf will still include `relative.conf`. + - The `createrawtransaction` RPC will now accept an array or dictionary (kept for compatibility) for the `outputs` parameter. This means the order of transaction outputs can be specified by the client. diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -386,10 +386,9 @@ if (request.fHelp || request.params.size() < 2 || request.params.size() > 3) { throw std::runtime_error( - "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] " - "{\"address\":amount,\"data\":\"hex\",...} ( locktime )\n" - "\nCreate a transaction spending the given inputs and creating new " - "outputs.\n" + // clang-format off + "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime )\n" + "\nCreate a transaction spending the given inputs and creating new outputs.\n" "Outputs can be addresses or data.\n" "Returns hex-encoded raw transaction.\n" "Note that the transaction's inputs are not signed, and\n" @@ -400,50 +399,41 @@ "json objects\n" " [\n" " {\n" - " \"txid\":\"id\", (string, required) The transaction " - "id\n" - " \"vout\":n, (numeric, required) The output " - "number\n" - " \"sequence\":n (numeric, optional) The sequence " - "number\n" + " \"txid\":\"id\", (string, required) The transaction id\n" + " \"vout\":n, (numeric, required) The output number\n" + " \"sequence\":n (numeric, optional) The sequence number\n" " } \n" " ,...\n" " ]\n" - "2. \"outputs\" (object, required) a json object " - "with outputs\n" + "2. \"outputs\" (array, required) a json array with outputs (key-value pairs)\n" + " [\n" " {\n" - " \"address\": x.xxx, (numeric or string, required) The " - "key is the bitcoin address, the numeric value (can be string) is " - "the " + - CURRENCY_UNIT + - " amount\n" - " \"data\": \"hex\" (string, required) The key is " - "\"data\", the value is hex encoded data\n" - " ,...\n" + " \"address\": x.xxx, (obj, optional) A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + "\n" + " },\n" + " {\n" + " \"data\": \"hex\" (obj, optional) A key-value pair. The key must be \"data\", the value is hex encoded data\n" " }\n" - "3. locktime (numeric, optional, default=0) Raw " - "locktime. Non-0 value also locktime-activates inputs\n" + " ,... More key-value pairs of the above form. For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" + " accepted as second parameter.\n" + " ]\n" + "3. locktime (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs\n" "\nResult:\n" - "\"transaction\" (string) hex string of the " - "transaction\n" + "\"transaction\" (string) hex string of the transaction\n" - "\nExamples:\n" + - HelpExampleCli("createrawtransaction", - "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" " - "\"{\\\"address\\\":0.01}\"") + - HelpExampleCli("createrawtransaction", - "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" " - "\"{\\\"data\\\":\\\"00010203\\\"}\"") + - HelpExampleRpc("createrawtransaction", - "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", " - "\"{\\\"address\\\":0.01}\"") + - HelpExampleRpc("createrawtransaction", - "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", " - "\"{\\\"data\\\":\\\"00010203\\\"}\"")); + "\nExamples:\n" + + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"address\\\":0.01}]\"") + + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"data\\\":\\\"00010203\\\"}]\"") + + HelpExampleRpc("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", \"[{\\\"address\\\":0.01}]\"") + + HelpExampleRpc("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", \"[{\\\"data\\\":\\\"00010203\\\"}]\"") + // clang-format on + ); } RPCTypeCheck(request.params, - {UniValue::VARR, UniValue::VOBJ, UniValue::VNUM}, true); + {UniValue::VARR, + UniValueType(), // ARR or OBJ, checked later + UniValue::VNUM, UniValue::VBOOL}, + true); if (request.params[0].isNull() || request.params[1].isNull()) { throw JSONRPCError( RPC_INVALID_PARAMETER, @@ -451,7 +441,9 @@ } UniValue inputs = request.params[0].get_array(); - UniValue sendTo = request.params[1].get_obj(); + const bool outputs_is_obj = request.params[1].isObject(); + UniValue outputs = outputs_is_obj ? request.params[1].get_obj() + : request.params[1].get_array(); CMutableTransaction rawTx; @@ -510,11 +502,29 @@ } std::set destinations; - std::vector addrList = sendTo.getKeys(); - for (const std::string &name_ : addrList) { + if (!outputs_is_obj) { + // Translate array of key-value pairs into dict + UniValue outputs_dict = UniValue(UniValue::VOBJ); + for (size_t i = 0; i < outputs.size(); ++i) { + const UniValue &output = outputs[i]; + if (!output.isObject()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "Invalid parameter, key-value pair not an " + "object as expected"); + } + if (output.size() != 1) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "Invalid parameter, key-value pair must " + "contain exactly one key"); + } + outputs_dict.pushKVs(output); + } + outputs = std::move(outputs_dict); + } + for (const std::string &name_ : outputs.getKeys()) { if (name_ == "data") { std::vector data = - ParseHexV(sendTo[name_].getValStr(), "Data"); + ParseHexV(outputs[name_].getValStr(), "Data"); CTxOut out(Amount::zero(), CScript() << OP_RETURN << data); rawTx.vout.push_back(out); @@ -535,7 +545,7 @@ } CScript scriptPubKey = GetScriptForDestination(destination); - Amount nAmount = AmountFromValue(sendTo[name_]); + Amount nAmount = AmountFromValue(outputs[name_]); CTxOut out(nAmount, scriptPubKey); rawTx.vout.push_back(out); diff --git a/src/rpc/server.h b/src/rpc/server.h --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -39,11 +39,10 @@ /** * Wrapper for UniValue::VType, which includes typeAny: used to denote don't - * care type. Only used by RPCTypeCheckObj. + * care type. */ struct UniValueType { - explicit UniValueType(UniValue::VType _type) - : typeAny(false), type(_type) {} + UniValueType(UniValue::VType _type) : typeAny(false), type(_type) {} UniValueType() : typeAny(true) {} bool typeAny; UniValue::VType type; @@ -101,13 +100,14 @@ * correct type. */ void RPCTypeCheck(const UniValue ¶ms, - const std::list &typesExpected, + const std::list &typesExpected, bool fAllowNull = false); /** * Type-check one argument; throws JSONRPCError if wrong type given. */ -void RPCTypeCheckArgument(const UniValue &value, UniValue::VType typeExpected); +void RPCTypeCheckArgument(const UniValue &value, + const UniValueType &typeExpected); /** * Check for expected keys/value types in an Object. diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -88,10 +88,10 @@ } void RPCTypeCheck(const UniValue ¶ms, - const std::list &typesExpected, + const std::list &typesExpected, bool fAllowNull) { unsigned int i = 0; - for (UniValue::VType t : typesExpected) { + for (const UniValueType &t : typesExpected) { if (params.size() <= i) { break; } @@ -104,11 +104,13 @@ } } -void RPCTypeCheckArgument(const UniValue &value, UniValue::VType typeExpected) { - if (value.type() != typeExpected) { - throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected type %s, got %s", - uvTypeName(typeExpected), - uvTypeName(value.type()))); +void RPCTypeCheckArgument(const UniValue &value, + const UniValueType &typeExpected) { + if (!typeExpected.typeAny && value.type() != typeExpected.type) { + throw JSONRPCError(RPC_TYPE_ERROR, + strprintf("Expected type %s, got %s", + uvTypeName(typeExpected.type), + uvTypeName(value.type()))); } } diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -53,8 +53,6 @@ std::runtime_error); BOOST_CHECK_THROW(CallRPC("createrawtransaction not_array"), std::runtime_error); - BOOST_CHECK_THROW(CallRPC("createrawtransaction [] []"), - std::runtime_error); BOOST_CHECK_THROW(CallRPC("createrawtransaction {} {}"), std::runtime_error); BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [] {}")); diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2014-2016 The Bitcoin Core developers +# Copyright (c) 2014-2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """rawtranscation RPCs QA test. @@ -13,6 +13,8 @@ """ from decimal import Decimal +from collections import OrderedDict +from io import BytesIO from test_framework.test_framework import BitcoinTestFramework from test_framework.txtools import pad_raw_tx from test_framework.util import ( @@ -20,6 +22,11 @@ assert_greater_than, assert_raises_rpc_error, connect_nodes_bi, + hex_str_to_bytes, + bytes_to_hex_str, +) +from test_framework.messages import ( + CTransaction, ) @@ -50,7 +57,8 @@ connect_nodes_bi(self.nodes[0], self.nodes[2]) def run_test(self): - # prepare some coins for multiple *rawtransaction commands + self.log.info( + 'prepare some coins for multiple *rawtransaction commands') self.nodes[2].generate(1) self.sync_all() self.nodes[0].generate(101) @@ -62,11 +70,14 @@ self.nodes[0].generate(5) self.sync_all() - # Test getrawtransaction on genesis block coinbase returns an error + self.log.info( + 'Test getrawtransaction on genesis block coinbase returns an error') block = self.nodes[0].getblock(self.nodes[0].getblockhash(0)) assert_raises_rpc_error(-5, "The genesis block coinbase is not considered an ordinary transaction", self.nodes[0].getrawtransaction, block['merkleroot']) + self.log.info( + 'Check parameter types and required parameters of createrawtransaction') # Test `createrawtransaction` required parameters assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction) @@ -98,8 +109,12 @@ # Test `createrawtransaction` invalid `outputs` address = self.nodes[0].getnewaddress() - assert_raises_rpc_error(-3, "Expected type object", + address2 = self.nodes[0].getnewaddress() + assert_raises_rpc_error(-1, "JSON value is not an array as expected", self.nodes[0].createrawtransaction, [], 'foo') + # Should not throw for backwards compatibility + self.nodes[0].createrawtransaction(inputs=[], outputs={}) + self.nodes[0].createrawtransaction(inputs=[], outputs=[]) assert_raises_rpc_error(-8, "Data must be hexadecimal string", self.nodes[0].createrawtransaction, [], {'data': 'foo'}) assert_raises_rpc_error(-5, "Invalid Bitcoin address", @@ -110,6 +125,12 @@ self.nodes[0].createrawtransaction, [], {address: -1}) assert_raises_rpc_error(-8, "Invalid parameter, duplicated address: {}".format( address), self.nodes[0].createrawtransaction, [], multidict([(address, 1), (address, 1)])) + assert_raises_rpc_error(-8, "Invalid parameter, duplicated address: {}".format( + address), self.nodes[0].createrawtransaction, [], [{address: 1}, {address: 1}]) + assert_raises_rpc_error(-8, "Invalid parameter, key-value pair must contain exactly one key", + self.nodes[0].createrawtransaction, [], [{'a': 1, 'b': 2}]) + assert_raises_rpc_error(-8, "Invalid parameter, key-value pair not an object as expected", + self.nodes[0].createrawtransaction, [], [['key-value pair1'], ['2']]) # Test `createrawtransaction` invalid `locktime` assert_raises_rpc_error(-3, "Expected type number", @@ -119,12 +140,50 @@ assert_raises_rpc_error(-8, "Invalid parameter, locktime out of range", self.nodes[0].createrawtransaction, [], {}, 4294967296) - # - # sendrawtransaction with missing input # - # + self.log.info( + 'Check that createrawtransaction accepts an array and object as outputs') + tx = CTransaction() + # One output + tx.deserialize(BytesIO(hex_str_to_bytes(self.nodes[2].createrawtransaction( + inputs=[{'txid': txid, 'vout': 9}], outputs={address: 99})))) + assert_equal(len(tx.vout), 1) + assert_equal( + bytes_to_hex_str(tx.serialize()), + self.nodes[2].createrawtransaction( + inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}]), + ) + # Two outputs + tx.deserialize(BytesIO(hex_str_to_bytes(self.nodes[2].createrawtransaction(inputs=[ + {'txid': txid, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)]))))) + assert_equal(len(tx.vout), 2) + assert_equal( + bytes_to_hex_str(tx.serialize()), + self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[ + {address: 99}, {address2: 99}]), + ) + # Two data outputs + tx.deserialize(BytesIO(hex_str_to_bytes(self.nodes[2].createrawtransaction(inputs=[ + {'txid': txid, 'vout': 9}], outputs=multidict([('data', '99'), ('data', '99')]))))) + assert_equal(len(tx.vout), 2) + assert_equal( + bytes_to_hex_str(tx.serialize()), + self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[ + {'data': '99'}, {'data': '99'}]), + ) + # Multiple mixed outputs + tx.deserialize(BytesIO(hex_str_to_bytes(self.nodes[2].createrawtransaction(inputs=[ + {'txid': txid, 'vout': 9}], outputs=multidict([(address, 99), ('data', '99'), ('data', '99')]))))) + assert_equal(len(tx.vout), 3) + assert_equal( + bytes_to_hex_str(tx.serialize()), + self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[ + {address: 99}, {'data': '99'}, {'data': '99'}]), + ) + + self.log.info('sendrawtransaction with missing input') + # won't exists inputs = [ {'txid': "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout': 1}] - # won't exists outputs = {self.nodes[0].getnewaddress(): 4.998} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) rawtx = pad_raw_tx(rawtx) @@ -306,18 +365,18 @@ rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs) rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet( rawTx2, inputs) - self.log.info(rawTxPartialSigned1) + self.log.debug(rawTxPartialSigned1) # node1 only has one key, can't comp. sign the tx assert_equal(rawTxPartialSigned['complete'], False) rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet( rawTx2, inputs) - self.log.info(rawTxPartialSigned2) + self.log.debug(rawTxPartialSigned2) # node2 only has one key, can't comp. sign the tx assert_equal(rawTxPartialSigned2['complete'], False) rawTxComb = self.nodes[2].combinerawtransaction( [rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) - self.log.info(rawTxComb) + self.log.debug(rawTxComb) self.nodes[2].sendrawtransaction(rawTxComb) rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb) self.sync_all()