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 (#4380)

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

Co-authored-by: MarcoFalke <falke.marco@gmail.com>
  • Loading branch information
@dzutto
dzutto and MarcoFalke committed Sep 1, 2021
1 parent bbc8623 commit 6d90e71
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
Expand Up @@ -22,11 +22,6 @@
#include <util/strencodings.h>
#include <validation.h>
#include <util/validation.h>
#ifdef ENABLE_WALLET
#include <wallet/rpcwallet.h>
#include <wallet/wallet.h>
#include <wallet/walletdb.h>
#endif
#include <warnings.h>

#include <masternode/masternode-sync.h>
Expand Down Expand Up @@ -220,29 +215,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));;
CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey));;

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() > 2)
Expand Down Expand Up @@ -282,8 +266,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 @@ -1141,7 +1124,7 @@ static const CRPCCommand commands[] =
{ "control", "debug", &debug, {} },
{ "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
118 changes: 9 additions & 109 deletions src/rpc/rawtransaction.cpp
View file
Expand Up @@ -32,9 +32,6 @@
#include <util/strencodings.h>
#include <validation.h>
#include <validationinterface.h>
#ifdef ENABLE_WALLET
#include <wallet/rpcwallet.h>
#endif

#include <evo/specialtx.h>
#include <evo/providertx.h>
Expand Down Expand Up @@ -837,8 +834,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()) {
RPCTypeCheckObj(prevOut,
{
Expand Down Expand Up @@ -1009,106 +1005,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) 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 dashd 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();

// This is needed to ensure that signrawtransactionwithwallet can figure out what wallet to use when multiple
// wallets are loaded
new_request.URI = request.URI;

// 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");
}

UniValue sendrawtransaction(const JSONRPCRequest& request)
Expand All @@ -1117,7 +1017,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
throw std::runtime_error(
"sendrawtransaction \"hexstring\" ( allowhighfees instantsend bypasslimits)\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 @@ -1129,7 +1029,7 @@ 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 @@ -1183,7 +1083,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 @@ -1750,7 +1650,7 @@ static const CRPCCommand commands[] =
{ "rawtransactions", "decodescript", &decodescript, {"hexstring"} },
{ "rawtransactions", "sendrawtransaction", &sendrawtransaction, {"hexstring","allowhighfees","instantsend","bypasslimits"} },
{ "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
Expand Up @@ -19,13 +19,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 = "yZNRHJXRPAiSMXd2knNE174gFqYKFbwVvB" # 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

self.log.info("Test accounts deprecation")
# The following account RPC methods are deprecated:
Expand Down
8 changes: 0 additions & 8 deletions test/functional/rpc_signrawtransaction.py
View file
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 test_with_lock_outputs(self):
"""Test correct error reporting when trying to sign a locked output"""
self.nodes[0].encryptwallet("password")
Expand Down Expand Up @@ -118,10 +114,6 @@ def script_verification_error_test(self):
assert_equal(rawTxSigned['errors'][1]['txid'], inputs[2]['txid'])
assert_equal(rawTxSigned['errors'][1]['vout'], inputs[2]['vout'])

# Perform same test with signrawtransaction
rawTxSigned2 = self.nodes[0].signrawtransaction(rawTx, scripts, privKeys)
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
Expand Up @@ -24,7 +24,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel"
"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/fees -> wallet/wallet -> wallet/fees"
Expand Down

0 comments on commit 6d90e71

Please sign in to comment.