From 67857bce60c677dbdcc734200678f9c1b8bd7d86 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 7 Dec 2018 17:19:28 +0100 Subject: [PATCH 1/2] Merge #14890: rpc: Avoid creating non-standard raw transactions fa4c8679ed94f215ce895938f7c3c169a2ce101e rpc: Avoid creating non-standard raw transactions (MarcoFalke) Pull request description: Multiple OP_RETURN outputs in a transaction are not standard and unlikely to be relayed, so avoid creating them. Apart from that, the logic was broken in that it duplicated the same hex-data for each data output: Closes #14868. Tree-SHA512: b08d08062b5622e8a7b497e490ccaf53b06e844c863fda3bf3f932a98684a809e8341aeb98232059a795afb32d8770a6c5591a66f8e6ee372b672af245607887 --- src/rpc/rawtransaction.cpp | 5 +++-- src/rpc/rawtransaction_util.cpp | 10 +++++++++- src/test/rpc_tests.cpp | 3 --- src/wallet/rpcwallet.cpp | 3 ++- test/functional/rpc_rawtransaction.py | 13 ++++--------- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 86da263a5074..7c2a46915cc5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -421,7 +421,7 @@ static UniValue createrawtransaction(const JSONRPCRequest& request) }, }, }, - {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs).\n" + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs), where none of the keys are duplicated.\n" "That is, each address can only appear once and there can only be one 'data' object.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" " accepted as second parameter.", @@ -1283,7 +1283,8 @@ UniValue createpsbt(const JSONRPCRequest& request) }, }, }, - {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs).\n" + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs), where none of the keys are duplicated.\n" + "That is, each address can only appear once and there can only be one 'data' object.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" " accepted as second parameter.", { diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 2c1f35248a8f..d58ac7cf574b 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -66,7 +66,6 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal rawTx.vin.push_back(in); } - std::set destinations; if (!outputs_is_obj) { // Translate array of key-value pairs into dict UniValue outputs_dict = UniValue(UniValue::VOBJ); @@ -82,8 +81,17 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal } outputs = std::move(outputs_dict); } + + // Duplicate checking + std::set destinations; + bool has_data{false}; + for (const std::string& name_ : outputs.getKeys()) { if (name_ == "data") { + if (has_data) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, duplicate key: data"); + } + has_data = true; std::vector data = ParseHexV(outputs[name_].getValStr(), "Data"); CTxOut out(0, CScript() << OP_RETURN << data); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 566b03e3d7e4..c2d0360fad1b 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -124,9 +124,6 @@ BOOST_AUTO_TEST_CASE(rpc_createraw_op_return) { BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [{\"txid\":\"a3b807410df0b60fcb9736768df5823938b2f838694939ba45f3c0a1bff150ed\",\"vout\":0}] {\"data\":\"68656c6c6f776f726c64\"}")); - // Allow more than one data transaction output - BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [{\"txid\":\"a3b807410df0b60fcb9736768df5823938b2f838694939ba45f3c0a1bff150ed\",\"vout\":0}] {\"data\":\"68656c6c6f776f726c64\",\"data\":\"68656c6c6f776f726c64\"}")); - // Key not "data" (bad address) BOOST_CHECK_THROW(CallRPC("createrawtransaction [{\"txid\":\"a3b807410df0b60fcb9736768df5823938b2f838694939ba45f3c0a1bff150ed\",\"vout\":0}] {\"somedata\":\"68656c6c6f776f726c64\"}"), std::runtime_error); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index da4ff7a8a2c5..8215e1cbd861 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4041,7 +4041,8 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) }, }, }, - {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs).\n" + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "a json array with outputs (key-value pairs), where none of the keys are duplicated.\n" + "That is, each address can only appear once and there can only be one 'data' object.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" " accepted as second parameter.", { diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 6bf015882423..98d926ebea91 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -109,6 +109,8 @@ def run_test(self): 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, duplicate key: data", self.nodes[0].createrawtransaction, [], [{"data": 'aa'}, {"data": "bb"}]) + assert_raises_rpc_error(-8, "Invalid parameter, duplicate key: data", self.nodes[0].createrawtransaction, [], multidict([("data", 'aa'), ("data", "bb")])) 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']]) @@ -133,19 +135,12 @@ def run_test(self): tx.serialize().hex(), 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( - tx.serialize().hex(), - 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')]))))) + tx.deserialize(BytesIO(hex_str_to_bytes(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')]))))) assert_equal(len(tx.vout), 3) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {'data': '99'}, {'data': '99'}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]), ) self.log.info('sendrawtransaction with missing input') From 4711bdba6b6c02c236861bb4d48c132909f28fa8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 13 Feb 2019 15:49:10 -0500 Subject: [PATCH 2/2] Merge #15390: [wallet-tool] Close bdb when flushing wallet 318b1f7af1 [wallet] Close bdb when flushing wallet. (John Newbery) Pull request description: bdb would not be closed when closing the wallet in wallet-tool. Fix this by calling wallet->flush with true. Tree-SHA512: f722e527e4806eca5254221e944f57853d11bf89a9264309fa558a6cc2b23feefb7bb2963e87b4fad9cfb31ac4cffe563688988e0614a481a8ff1d393aceb132 --- src/wallet/wallettool.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index bf9278470421..877c441c882f 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -17,7 +17,7 @@ namespace WalletTool { static void WalletToolReleaseWallet(CWallet* wallet) { wallet->WalletLogPrintf("Releasing wallet\n"); - wallet->Flush(); + wallet->Flush(true); delete wallet; } @@ -114,7 +114,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) std::shared_ptr wallet_instance = CreateWallet(name, path); if (wallet_instance) { WalletShowInfo(wallet_instance.get()); - wallet_instance->Flush(); + wallet_instance->Flush(true); } } else if (command == "info") { if (!fs::exists(path)) { @@ -129,7 +129,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) std::shared_ptr wallet_instance = LoadWallet(name, path); if (!wallet_instance) return false; WalletShowInfo(wallet_instance.get()); - wallet_instance->Flush(); + wallet_instance->Flush(true); } else { tfm::format(std::cerr, "Invalid command: %s\n", command); return false;