Merge #11872: [rpc] createrawtransaction: Accept sorted outputs
fac70134arpc: Update createrawtransaction examples (MarcoFalke)fa06dfce0[rpc] createrawtransaction: Accept sorted outputs (MarcoFalke)8acd25d85rpc: Allow typeAny in RPCTypeCheck (MarcoFalke) Pull request description: The second parameter of the `createrawtransaction` is a dictionary of the outputs. This comes with at least two drawbacks: * In case of duplicate keys, either of them might silently disappear, with no user feedback at all. A user needs to make other mistakes, but this could eventually lead to abnormal tx fees. * A dictionary does not guarantee that keys are sorted. Again, a user needs to keep this in mind, as it could eventually lead to excessive tx fees. Even though my scenario of loss-of-funds is unlikely to happen, I see it as a inconvenience that should be fixed. Tree-SHA512: cd562f34f7f9f79c7d3433805971325c388c2035611be283980f4049066a622df4f0afdc11d7ac96662260ec0115147cb65e1ab5268f5a1b063242f3fe425f77
This commit is contained in:
Wladimir J. van der Laan
commit
702e8b70bd
6 changed files with 111 additions and 42 deletions
|
|
@ -61,7 +61,8 @@ RPC changes
|
|||
|
||||
### Low-level changes
|
||||
|
||||
- The `fundrawtransaction` rpc will reject the previously deprecated `reserveChangeKey` option.
|
||||
- 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.
|
||||
- The `fundrawtransaction` RPC will reject the previously deprecated `reserveChangeKey` option.
|
||||
|
||||
External wallet files
|
||||
---------------------
|
||||
|
|
|
|||
|
|
@ -316,9 +316,10 @@ UniValue verifytxoutproof(const JSONRPCRequest& request)
|
|||
|
||||
UniValue createrawtransaction(const JSONRPCRequest& request)
|
||||
{
|
||||
if (request.fHelp || request.params.size() < 2 || request.params.size() > 4)
|
||||
if (request.fHelp || request.params.size() < 2 || request.params.size() > 4) {
|
||||
throw std::runtime_error(
|
||||
"createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] {\"address\":amount,\"data\":\"hex\",...} ( locktime ) ( replaceable )\n"
|
||||
// clang-format off
|
||||
"createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime ) ( replaceable )\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"
|
||||
|
|
@ -329,18 +330,23 @@ UniValue createrawtransaction(const JSONRPCRequest& request)
|
|||
"1. \"inputs\" (array, required) A json array of json objects\n"
|
||||
" [\n"
|
||||
" {\n"
|
||||
" \"txid\":\"id\", (string, required) The transaction id\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"
|
||||
" ,... 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"
|
||||
"4. replaceable (boolean, optional, default=false) Marks this transaction as BIP125 replaceable.\n"
|
||||
" Allows this transaction to be replaced by a transaction with higher fees. If provided, it is an error if explicit sequence numbers are incompatible.\n"
|
||||
|
|
@ -348,18 +354,29 @@ UniValue createrawtransaction(const JSONRPCRequest& request)
|
|||
"\"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\\\"}\"")
|
||||
+ 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, UniValue::VBOOL}, true);
|
||||
RPCTypeCheck(request.params, {
|
||||
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, "Invalid parameter, arguments 1 and 2 must be non-null");
|
||||
|
||||
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;
|
||||
|
||||
|
|
@ -411,11 +428,24 @@ UniValue createrawtransaction(const JSONRPCRequest& request)
|
|||
}
|
||||
|
||||
std::set<CTxDestination> destinations;
|
||||
std::vector<std::string> 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<unsigned char> data = ParseHexV(sendTo[name_].getValStr(),"Data");
|
||||
std::vector<unsigned char> data = ParseHexV(outputs[name_].getValStr(), "Data");
|
||||
|
||||
CTxOut out(0, CScript() << OP_RETURN << data);
|
||||
rawTx.vout.push_back(out);
|
||||
|
|
@ -430,7 +460,7 @@ UniValue createrawtransaction(const JSONRPCRequest& request)
|
|||
}
|
||||
|
||||
CScript scriptPubKey = GetScriptForDestination(destination);
|
||||
CAmount nAmount = AmountFromValue(sendTo[name_]);
|
||||
CAmount nAmount = AmountFromValue(outputs[name_]);
|
||||
|
||||
CTxOut out(nAmount, scriptPubKey);
|
||||
rawTx.vout.push_back(out);
|
||||
|
|
|
|||
|
|
@ -50,12 +50,11 @@ void RPCServer::OnStopped(std::function<void ()> slot)
|
|||
}
|
||||
|
||||
void RPCTypeCheck(const UniValue& params,
|
||||
const std::list<UniValue::VType>& typesExpected,
|
||||
const std::list<UniValueType>& typesExpected,
|
||||
bool fAllowNull)
|
||||
{
|
||||
unsigned int i = 0;
|
||||
for (UniValue::VType t : typesExpected)
|
||||
{
|
||||
for (const UniValueType& t : typesExpected) {
|
||||
if (params.size() <= i)
|
||||
break;
|
||||
|
||||
|
|
@ -67,10 +66,10 @@ void RPCTypeCheck(const UniValue& params,
|
|||
}
|
||||
}
|
||||
|
||||
void RPCTypeCheckArgument(const UniValue& value, UniValue::VType typeExpected)
|
||||
void RPCTypeCheckArgument(const UniValue& value, const UniValueType& typeExpected)
|
||||
{
|
||||
if (value.type() != typeExpected) {
|
||||
throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected type %s, got %s", uvTypeName(typeExpected), uvTypeName(value.type())));
|
||||
if (!typeExpected.typeAny && value.type() != typeExpected.type) {
|
||||
throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected type %s, got %s", uvTypeName(typeExpected.type), uvTypeName(value.type())));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -28,9 +28,9 @@ namespace RPCServer
|
|||
}
|
||||
|
||||
/** Wrapper for UniValue::VType, which includes typeAny:
|
||||
* Used to denote don't care type. Only used by RPCTypeCheckObj */
|
||||
* Used to denote don't 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;
|
||||
|
|
@ -69,12 +69,12 @@ bool RPCIsInWarmup(std::string *outStatus);
|
|||
* the right number of arguments are passed, just that any passed are the correct type.
|
||||
*/
|
||||
void RPCTypeCheck(const UniValue& params,
|
||||
const std::list<UniValue::VType>& typesExpected, bool fAllowNull=false);
|
||||
const std::list<UniValueType>& 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.
|
||||
|
|
|
|||
|
|
@ -52,7 +52,6 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams)
|
|||
BOOST_CHECK_THROW(CallRPC("createrawtransaction"), std::runtime_error);
|
||||
BOOST_CHECK_THROW(CallRPC("createrawtransaction null null"), 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 [] {}"));
|
||||
BOOST_CHECK_THROW(CallRPC("createrawtransaction [] {} extra"), std::runtime_error);
|
||||
|
|
|
|||
|
|
@ -12,7 +12,12 @@ Test the following RPCs:
|
|||
- getrawtransaction
|
||||
"""
|
||||
|
||||
from collections import OrderedDict
|
||||
from io import BytesIO
|
||||
from test_framework.test_framework import BitcoinTestFramework
|
||||
from test_framework.messages import (
|
||||
CTransaction,
|
||||
)
|
||||
from test_framework.util import *
|
||||
|
||||
|
||||
|
|
@ -43,11 +48,10 @@ class RawTransactionsTest(BitcoinTestFramework):
|
|||
|
||||
def setup_network(self, split=False):
|
||||
super().setup_network()
|
||||
connect_nodes_bi(self.nodes,0,2)
|
||||
connect_nodes_bi(self.nodes, 0, 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)
|
||||
|
|
@ -59,10 +63,11 @@ class RawTransactionsTest(BitcoinTestFramework):
|
|||
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)
|
||||
assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [])
|
||||
|
|
@ -83,12 +88,18 @@ class RawTransactionsTest(BitcoinTestFramework):
|
|||
|
||||
# Test `createrawtransaction` invalid `outputs`
|
||||
address = self.nodes[0].getnewaddress()
|
||||
assert_raises_rpc_error(-3, "Expected type object", self.nodes[0].createrawtransaction, [], 'foo')
|
||||
address2 = self.nodes[0].getnewaddress()
|
||||
assert_raises_rpc_error(-1, "JSON value is not an array as expected", self.nodes[0].createrawtransaction, [], 'foo')
|
||||
self.nodes[0].createrawtransaction(inputs=[], outputs={}) # Should not throw for backwards compatibility
|
||||
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", self.nodes[0].createrawtransaction, [], {'foo': 0})
|
||||
assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].createrawtransaction, [], {address: 'foo'})
|
||||
assert_raises_rpc_error(-3, "Amount out of range", self.nodes[0].createrawtransaction, [], {address: -1})
|
||||
assert_raises_rpc_error(-8, "Invalid parameter, duplicated address: %s" % address, self.nodes[0].createrawtransaction, [], multidict([(address, 1), (address, 1)]))
|
||||
assert_raises_rpc_error(-8, "Invalid parameter, duplicated address: %s" % 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", self.nodes[0].createrawtransaction, [], {}, 'foo')
|
||||
|
|
@ -98,9 +109,38 @@ class RawTransactionsTest(BitcoinTestFramework):
|
|||
# Test `createrawtransaction` invalid `replaceable`
|
||||
assert_raises_rpc_error(-3, "Expected type bool", self.nodes[0].createrawtransaction, [], {}, 0, 'foo')
|
||||
|
||||
#########################################
|
||||
# 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')
|
||||
inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1}] #won't exists
|
||||
outputs = { self.nodes[0].getnewaddress() : 4.998 }
|
||||
rawtx = self.nodes[2].createrawtransaction(inputs, outputs)
|
||||
|
|
@ -248,14 +288,14 @@ class RawTransactionsTest(BitcoinTestFramework):
|
|||
outputs = { self.nodes[0].getnewaddress() : 2.19 }
|
||||
rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs)
|
||||
rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs)
|
||||
self.log.info(rawTxPartialSigned1)
|
||||
self.log.debug(rawTxPartialSigned1)
|
||||
assert_equal(rawTxPartialSigned['complete'], False) #node1 only has one key, can't comp. sign the tx
|
||||
|
||||
rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs)
|
||||
self.log.info(rawTxPartialSigned2)
|
||||
self.log.debug(rawTxPartialSigned2)
|
||||
assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx
|
||||
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()
|
||||
|
|
@ -273,7 +313,7 @@ class RawTransactionsTest(BitcoinTestFramework):
|
|||
encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000"
|
||||
decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction
|
||||
assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
|
||||
|
||||
|
||||
# getrawtransaction tests
|
||||
# 1. valid parameters - only supply txid
|
||||
txHash = rawTx["hash"]
|
||||
|
|
|
|||
Loading…
Reference in a new issue