New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[rpc] createrawtransaction: Accept sorted outputs #11872
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,37 +330,53 @@ 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" | ||
"\nResult:\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\\\"}\"") | ||
+ 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]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate output is an object. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Also added a test for this |
||
if (!output.isObject()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would make the error message less specific There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right. |
||
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); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move to different commit? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thx, done. |
||
{ | ||
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()))); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just change from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thx. Moved the check to the functional tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not change to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jnewbery I think we deprecated the existing rpc unit tests in favor of the functional tests which test the rpc interface. If you wanted reasonable coverage, you'd have to duplicate all the rpc unit tests based on the functional rpc tests. So yeah, I am not convinced that the unit tests add additional coverage... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you really think the unit tests are deprecated, they should be removed in a separate PR. It doesn't make sense to me to remove just this one case in this PR. Anyway, as I said - I don't think this should hold up merge. I'm just puzzled that you'd chose a larger diff that reduces test coverage (the fact that this unit test failed in the first iteration of this PR tells me that it was testing something). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Moving all those tests to the functional test suite makes sense. |
||
BOOST_CHECK_THROW(CallRPC("createrawtransaction {} {}"), std::runtime_error); | ||
BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [] {}")); | ||
BOOST_CHECK_THROW(CallRPC("createrawtransaction [] {} extra"), std::runtime_error); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, the indentation could be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alignment too:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed alignment