Skip to content

Commit

Permalink
Merge bitcoin#12490: [Wallet] [RPC] Remove deprecated wallet rpc feat…
Browse files Browse the repository at this point in the history 
…ures from bitcoin_server

f7e9e70 [rpc] Remove deprecated sigrawtransaction rpc method. (John Newbery)
90c8340 [RPC] Remove warning about wallet addresses in createmultisig() (John Newbery)
df905e3 [rpc] Remove deprecated validateaddress usage. (John Newbery)

Pull request description:

  The following rpc features were deprecated in V0.17:

  - `validateaddress` returning wallet information about an address
  - `signrawtransaction`

  This PR fully removes those features. It can be merged once V0.17 has been branched from master.

Tree-SHA512: 28293d218cf7e348632081e362f8775f243d091f49aed54c354f017d4a12ae92b87b99f81ee592a1bbf4aebd5d8cd5119278141edde7a0399ff82917ed68b9f6
  • Loading branch information
MarcoFalke committed Sep 7, 2018
2 parents 2f7ae35 + f7e9e70 commit 4799b09
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 150 deletions.
33 changes: 8 additions & 25 deletions src/rpc/misc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@
#include <timedata.h>
#include <util.h>
#include <utilstrencodings.h>
#ifdef ENABLE_WALLET
#include <wallet/rpcwallet.h>
#include <wallet/wallet.h>
#include <wallet/walletdb.h>
#endif
#include <warnings.h>

#include <stdint.h>
Expand Down Expand Up @@ -67,29 +62,18 @@ static UniValue validateaddress(const JSONRPCRequest& request)
ret.pushKV("isvalid", isValid);
if (isValid)
{
std::string currentAddress = EncodeDestination(dest);
ret.pushKV("address", currentAddress);

#ifdef ENABLE_WALLET
if (HasWallets() && IsDeprecatedRPCEnabled("validateaddress")) {
ret.pushKVs(getaddressinfo(request));
}
#endif
if (ret["address"].isNull()) {
std::string currentAddress = EncodeDestination(dest);
ret.pushKV("address", currentAddress);

CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));
CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end()));

UniValue detail = DescribeAddress(dest);
ret.pushKVs(detail);
}
UniValue detail = DescribeAddress(dest);
ret.pushKVs(detail);
}
return ret;
}

// Needed even with !ENABLE_WALLET, to pass (ignored) pointers around
class CWallet;

static UniValue createmultisig(const JSONRPCRequest& request)
{
if (request.fHelp || request.params.size() < 2 || request.params.size() > 3)
Expand Down Expand Up @@ -130,8 +114,7 @@ static UniValue createmultisig(const JSONRPCRequest& request)
if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) {
pubkeys.push_back(HexToPubKey(keys[i].get_str()));
} else {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\nNote that from v0.16, createmultisig no longer accepts addresses."
" Users must use addmultisigaddress to create multisig addresses with addresses known to the wallet.", keys[i].get_str()));
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\n.", keys[i].get_str()));
}
}

Expand Down Expand Up @@ -461,7 +444,7 @@ static const CRPCCommand commands[] =
// --------------------- ------------------------ ----------------------- ----------
{ "control", "getmemoryinfo", &getmemoryinfo, {"mode"} },
{ "control", "logging", &logging, {"include", "exclude"}},
{ "util", "validateaddress", &validateaddress, {"address"} }, /* uses wallet if enabled */
{ "util", "validateaddress", &validateaddress, {"address"} },
{ "util", "createmultisig", &createmultisig, {"nrequired","keys"} },
{ "util", "verifymessage", &verifymessage, {"address","signature","message"} },
{ "util", "signmessagewithprivkey", &signmessagewithprivkey, {"privkey","message"} },
Expand Down
114 changes: 9 additions & 105 deletions src/rpc/rawtransaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@
#include <txmempool.h>
#include <uint256.h>
#include <utilstrencodings.h>
#ifdef ENABLE_WALLET
#include <wallet/rpcwallet.h>
#endif

#include <future>
#include <stdint.h>
Expand Down Expand Up @@ -824,8 +821,7 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
view.AddCoin(out, std::move(newcoin), true);
}

// if redeemScript given and not using the local wallet (private keys
// given), add redeemScript to the keystore so it can be signed:
// if redeemScript and private keys were given, add redeemScript to the keystore so it can be signed
if (is_temp_keystore && (scriptPubKey.IsPayToScriptHash() || scriptPubKey.IsPayToWitnessScriptHash())) {
RPCTypeCheckObj(prevOut,
{
Expand Down Expand Up @@ -980,102 +976,10 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request)

UniValue signrawtransaction(const JSONRPCRequest& request)
{
#ifdef ENABLE_WALLET
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
CWallet* const pwallet = wallet.get();
#endif

if (request.fHelp || request.params.size() < 1 || request.params.size() > 4)
throw std::runtime_error(
"signrawtransaction \"hexstring\" ( [{\"txid\":\"id\",\"vout\":n,\"scriptPubKey\":\"hex\",\"redeemScript\":\"hex\"},...] [\"privatekey1\",...] sighashtype )\n"
"\nDEPRECATED. Sign inputs for raw transaction (serialized, hex-encoded).\n"
"The second optional argument (may be null) is an array of previous transaction outputs that\n"
"this transaction depends on but may not yet be in the block chain.\n"
"The third optional argument (may be null) is an array of base58-encoded private\n"
"keys that, if given, will be the only keys used to sign the transaction.\n"
#ifdef ENABLE_WALLET
+ HelpRequiringPassphrase(pwallet) + "\n"
#endif
"\nArguments:\n"
"1. \"hexstring\" (string, required) The transaction hex string\n"
"2. \"prevtxs\" (string, optional) An json array of previous dependent transaction outputs\n"
" [ (json array of json objects, or 'null' if none provided)\n"
" {\n"
" \"txid\":\"id\", (string, required) The transaction id\n"
" \"vout\":n, (numeric, required) The output number\n"
" \"scriptPubKey\": \"hex\", (string, required) script key\n"
" \"redeemScript\": \"hex\", (string, required for P2SH or P2WSH) redeem script\n"
" \"amount\": value (numeric, required) The amount spent\n"
" }\n"
" ,...\n"
" ]\n"
"3. \"privkeys\" (string, optional) A json array of base58-encoded private keys for signing\n"
" [ (json array of strings, or 'null' if none provided)\n"
" \"privatekey\" (string) private key in base58-encoding\n"
" ,...\n"
" ]\n"
"4. \"sighashtype\" (string, optional, default=ALL) The signature hash type. Must be one of\n"
" \"ALL\"\n"
" \"NONE\"\n"
" \"SINGLE\"\n"
" \"ALL|ANYONECANPAY\"\n"
" \"NONE|ANYONECANPAY\"\n"
" \"SINGLE|ANYONECANPAY\"\n"

"\nResult:\n"
"{\n"
" \"hex\" : \"value\", (string) The hex-encoded raw transaction with signature(s)\n"
" \"complete\" : true|false, (boolean) If the transaction has a complete set of signatures\n"
" \"errors\" : [ (json array of objects) Script verification errors (if there are any)\n"
" {\n"
" \"txid\" : \"hash\", (string) The hash of the referenced, previous transaction\n"
" \"vout\" : n, (numeric) The index of the output to spent and used as input\n"
" \"scriptSig\" : \"hex\", (string) The hex-encoded signature script\n"
" \"sequence\" : n, (numeric) Script sequence number\n"
" \"error\" : \"text\" (string) Verification or signing error related to the input\n"
" }\n"
" ,...\n"
" ]\n"
"}\n"

"\nExamples:\n"
+ HelpExampleCli("signrawtransaction", "\"myhex\"")
+ HelpExampleRpc("signrawtransaction", "\"myhex\"")
);

if (!IsDeprecatedRPCEnabled("signrawtransaction")) {
throw JSONRPCError(RPC_METHOD_DEPRECATED, "signrawtransaction is deprecated and will be fully removed in v0.18. "
"To use signrawtransaction in v0.17, restart bitcoind with -deprecatedrpc=signrawtransaction.\n"
"Projects should transition to using signrawtransactionwithkey and signrawtransactionwithwallet before upgrading to v0.18");
}

RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VARR, UniValue::VSTR}, true);

// Make a JSONRPCRequest to pass on to the right signrawtransaction* command
JSONRPCRequest new_request;
new_request.id = request.id;
new_request.params.setArray();

// For signing with private keys
if (!request.params[2].isNull()) {
new_request.params.push_back(request.params[0]);
// Note: the prevtxs and privkeys are reversed for signrawtransactionwithkey
new_request.params.push_back(request.params[2]);
new_request.params.push_back(request.params[1]);
new_request.params.push_back(request.params[3]);
return signrawtransactionwithkey(new_request);
} else {
#ifdef ENABLE_WALLET
// Otherwise sign with the wallet which does not take a privkeys parameter
new_request.params.push_back(request.params[0]);
new_request.params.push_back(request.params[1]);
new_request.params.push_back(request.params[3]);
return signrawtransactionwithwallet(new_request);
#else
// If we have made it this far, then wallet is disabled and no private keys were given, so fail here.
throw JSONRPCError(RPC_INVALID_PARAMETER, "No private keys available.");
#endif
}
// This method should be removed entirely in V0.19, along with the entries in the
// CRPCCommand table and rpc/client.cpp.
throw JSONRPCError(RPC_METHOD_DEPRECATED, "signrawtransaction was removed in v0.18.\n"
"Clients should transition to using signrawtransactionwithkey and signrawtransactionwithwallet");
}

static UniValue sendrawtransaction(const JSONRPCRequest& request)
Expand All @@ -1084,7 +988,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request)
throw std::runtime_error(
"sendrawtransaction \"hexstring\" ( allowhighfees )\n"
"\nSubmits raw transaction (serialized, hex-encoded) to local node and network.\n"
"\nAlso see createrawtransaction and signrawtransaction calls.\n"
"\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n"
"\nArguments:\n"
"1. \"hexstring\" (string, required) The hex string of the raw transaction)\n"
"2. allowhighfees (boolean, optional, default=false) Allow high fees\n"
Expand All @@ -1094,7 +998,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request)
"\nCreate a transaction\n"
+ HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") +
"Sign the transaction, and get back the hex\n"
+ HelpExampleCli("signrawtransaction", "\"myhex\"") +
+ HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") +
"\nSend the transaction (signed hex)\n"
+ HelpExampleCli("sendrawtransaction", "\"signedhex\"") +
"\nAs a json rpc call\n"
Expand Down Expand Up @@ -1199,7 +1103,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
"\nCreate a transaction\n"
+ HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") +
"Sign the transaction, and get back the hex\n"
+ HelpExampleCli("signrawtransaction", "\"myhex\"") +
+ HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") +
"\nTest acceptance of the transaction (signed hex)\n"
+ HelpExampleCli("testmempoolaccept", "\"signedhex\"") +
"\nAs a json rpc call\n"
Expand Down Expand Up @@ -1800,7 +1704,7 @@ static const CRPCCommand commands[] =
{ "rawtransactions", "decodescript", &decodescript, {"hexstring"} },
{ "rawtransactions", "sendrawtransaction", &sendrawtransaction, {"hexstring","allowhighfees"} },
{ "rawtransactions", "combinerawtransaction", &combinerawtransaction, {"txs"} },
{ "rawtransactions", "signrawtransaction", &signrawtransaction, {"hexstring","prevtxs","privkeys","sighashtype"} }, /* uses wallet if enabled */
{ "hidden", "signrawtransaction", &signrawtransaction, {"hexstring","prevtxs","privkeys","sighashtype"} },
{ "rawtransactions", "signrawtransactionwithkey", &signrawtransactionwithkey, {"hexstring","privkeys","prevtxs","sighashtype"} },
{ "rawtransactions", "testmempoolaccept", &testmempoolaccept, {"rawtxs","allowhighfees"} },
{ "rawtransactions", "decodepsbt", &decodepsbt, {"psbt"} },
Expand Down
8 changes: 1 addition & 7 deletions test/functional/rpc_deprecated.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,7 @@ def run_test(self):
# self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses")
# assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()])
# self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])

self.log.info("Test validateaddress deprecation")
SOME_ADDRESS = "mnvGjUy3NMj67yJ6gkK5o9e5RS33Z2Vqcu" # This is just some random address to pass as a parameter to validateaddress
dep_validate_address = self.nodes[0].validateaddress(SOME_ADDRESS)
assert "ismine" not in dep_validate_address
not_dep_val = self.nodes[1].validateaddress(SOME_ADDRESS)
assert "ismine" in not_dep_val
pass

if __name__ == '__main__':
DeprecatedRpcTest().main()
12 changes: 0 additions & 12 deletions test/functional/rpc_signrawtransaction.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ def successful_signing_test(self):
# 2) No script verification error occurred
assert 'errors' not in rawTxSigned

# Perform the same test on signrawtransaction
rawTxSigned2 = self.nodes[0].signrawtransaction(rawTx, inputs, privKeys)
assert_equal(rawTxSigned, rawTxSigned2)

def script_verification_error_test(self):
"""Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script.
Expand Down Expand Up @@ -112,10 +108,6 @@ def script_verification_error_test(self):
assert_equal(rawTxSigned['errors'][1]['vout'], inputs[2]['vout'])
assert not rawTxSigned['errors'][0]['witness']

# Perform same test with signrawtransaction
rawTxSigned2 = self.nodes[0].signrawtransaction(rawTx, scripts, privKeys)
assert_equal(rawTxSigned, rawTxSigned2)

# Now test signing failure for transaction with input witnesses
p2wpkh_raw_tx = "01000000000102fff7f7881a8099afa6940d42d1e7f6362bec38171ea3edf433541db4e4ad969f00000000494830450221008b9d1dc26ba6a9cb62127b02742fa9d754cd3bebf337f7a55d114c8e5cdd30be022040529b194ba3f9281a99f2b1c0a19c0489bc22ede944ccf4ecbab4cc618ef3ed01eeffffffef51e1b804cc89d182d279655c3aa89e815b1b309fe287d9b2b55d57b90ec68a0100000000ffffffff02202cb206000000001976a9148280b37df378db99f66f85c95a783a76ac7a6d5988ac9093510d000000001976a9143bde42dbee7e4dbe6a21b2d50ce2f0167faa815988ac000247304402203609e17b84f6a7d30c80bfa610b5b4542f32a8a0d5447a12fb1366d7f01cc44a0220573a954c4518331561406f90300e8f3358f51928d43c212a8caed02de67eebee0121025476c2e83188368da1ff3e292e7acafcdb3566bb0ad253f62fc70f07aeee635711000000"

Expand All @@ -140,10 +132,6 @@ def script_verification_error_test(self):
assert_equal(rawTxSigned['errors'][1]['witness'], ["304402203609e17b84f6a7d30c80bfa610b5b4542f32a8a0d5447a12fb1366d7f01cc44a0220573a954c4518331561406f90300e8f3358f51928d43c212a8caed02de67eebee01", "025476c2e83188368da1ff3e292e7acafcdb3566bb0ad253f62fc70f07aeee6357"])
assert not rawTxSigned['errors'][0]['witness']

# Perform same test with signrawtransaction
rawTxSigned2 = self.nodes[0].signrawtransaction(p2wpkh_raw_tx)
assert_equal(rawTxSigned, rawTxSigned2)

def run_test(self):
self.successful_signing_test()
self.script_verification_error_test()
Expand Down
1 change: 0 additions & 1 deletion test/lint/lint-circular-dependencies.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog"
"qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel"
"qt/walletmodel -> qt/walletmodeltransaction -> qt/walletmodel"
"rpc/rawtransaction -> wallet/rpcwallet -> rpc/rawtransaction"
"txmempool -> validation -> txmempool"
"validation -> validationinterface -> validation"
"wallet/coincontrol -> wallet/wallet -> wallet/coincontrol"
Expand Down

0 comments on commit 4799b09

Please sign in to comment.