Merge #9332: Let wallet importmulti RPC accept labels for standard scriptPubKeys

98ea64cf23 Let wallet importmulti RPC accept labels for standard scriptPubKeys (Russell Yanofsky)

Pull request description:

  Allow importmulti RPC to apply address labels when importing standard scriptPubKeys. This makes the importmulti RPC less finnicky about import formats and also simpler internally.

Tree-SHA512: 102426b21239f1fa5f38162dc3f4145572caef76e63906afd786b7aff1670d6cd93456f8d85f737588eedc49c11bef2e1e8019b8b2cbf6097c77b3501b0cab1f
This commit is contained in:
Wladimir J. van der Laan 2018-09-10 19:01:13 +02:00
commit 13c842e028
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
3 changed files with 28 additions and 51 deletions

View file

@ -849,6 +849,9 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
std::vector<unsigned char> vData(ParseHex(output)); std::vector<unsigned char> vData(ParseHex(output));
script = CScript(vData.begin(), vData.end()); script = CScript(vData.begin(), vData.end());
if (!ExtractDestination(script, dest) && !internal) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal must be set to true for nonstandard scriptPubKey imports.");
}
} }
// Watchonly and private keys // Watchonly and private keys
@ -861,11 +864,6 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between internal and label"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between internal and label");
} }
// Not having Internal + Script
if (!internal && isScript) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal must be set for hex scriptPubKey");
}
// Keys / PubKeys size check. // Keys / PubKeys size check.
if (!isP2SH && (keys.size() > 1 || pubKeys.size() > 1)) { // Address / scriptPubKey if (!isP2SH && (keys.size() > 1 || pubKeys.size() > 1)) { // Address / scriptPubKey
throw JSONRPCError(RPC_INVALID_PARAMETER, "More than private key given for one address"); throw JSONRPCError(RPC_INVALID_PARAMETER, "More than private key given for one address");
@ -969,21 +967,10 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
CTxDestination pubkey_dest = pubKey.GetID(); CTxDestination pubkey_dest = pubKey.GetID();
// Consistency check. // Consistency check.
if (!isScript && !(pubkey_dest == dest)) { if (!(pubkey_dest == dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed"); throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
} }
// Consistency check.
if (isScript) {
CTxDestination destination;
if (ExtractDestination(script, destination)) {
if (!(destination == pubkey_dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
}
}
}
CScript pubKeyScript = GetScriptForDestination(pubkey_dest); CScript pubKeyScript = GetScriptForDestination(pubkey_dest);
if (::IsMine(*pwallet, pubKeyScript) == ISMINE_SPENDABLE) { if (::IsMine(*pwallet, pubKeyScript) == ISMINE_SPENDABLE) {
@ -1034,21 +1021,10 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
CTxDestination pubkey_dest = pubKey.GetID(); CTxDestination pubkey_dest = pubKey.GetID();
// Consistency check. // Consistency check.
if (!isScript && !(pubkey_dest == dest)) { if (!(pubkey_dest == dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed"); throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
} }
// Consistency check.
if (isScript) {
CTxDestination destination;
if (ExtractDestination(script, destination)) {
if (!(destination == pubkey_dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
}
}
}
CKeyID vchAddress = pubKey.GetID(); CKeyID vchAddress = pubKey.GetID();
pwallet->MarkDirty(); pwallet->MarkDirty();
pwallet->SetAddressBook(vchAddress, label, "receive"); pwallet->SetAddressBook(vchAddress, label, "receive");
@ -1080,11 +1056,9 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
} }
if (scriptPubKey.getType() == UniValue::VOBJ) { // add to address book or update label
// add to address book or update label if (IsValidDestination(dest)) {
if (IsValidDestination(dest)) { pwallet->SetAddressBook(dest, label, "receive");
pwallet->SetAddressBook(dest, label, "receive");
}
} }
success = true; success = true;

View file

@ -26,7 +26,7 @@ import collections
import enum import enum
import itertools import itertools
Call = enum.Enum("Call", "single multi") Call = enum.Enum("Call", "single multiaddress multiscript")
Data = enum.Enum("Data", "address pub priv") Data = enum.Enum("Data", "address pub priv")
Rescan = enum.Enum("Rescan", "no yes late_timestamp") Rescan = enum.Enum("Rescan", "no yes late_timestamp")
@ -53,11 +53,11 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")):
response = self.try_rpc(self.node.importprivkey, privkey=self.key, rescan=rescan) response = self.try_rpc(self.node.importprivkey, privkey=self.key, rescan=rescan)
assert_equal(response, None) assert_equal(response, None)
elif self.call == Call.multi: elif self.call in (Call.multiaddress, Call.multiscript):
response = self.node.importmulti([{ response = self.node.importmulti([{
"scriptPubKey": { "scriptPubKey": {
"address": self.address["address"] "address": self.address["address"]
}, } if self.call == Call.multiaddress else self.address["scriptPubKey"],
"timestamp": timestamp + TIMESTAMP_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0), "timestamp": timestamp + TIMESTAMP_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0),
"pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [], "pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [],
"keys": [self.key] if self.data == Data.priv else [], "keys": [self.key] if self.data == Data.priv else [],
@ -126,7 +126,7 @@ class ImportRescanTest(BitcoinTestFramework):
for i, variant in enumerate(IMPORT_VARIANTS): for i, variant in enumerate(IMPORT_VARIANTS):
variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress()) variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress())
variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
variant.initial_amount = 10 - (i + 1) / 4.0 variant.initial_amount = 1 - (i + 1) / 64
variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
# Generate a block containing the initial transactions, then another # Generate a block containing the initial transactions, then another
@ -156,7 +156,7 @@ class ImportRescanTest(BitcoinTestFramework):
# Create new transactions sending to each address. # Create new transactions sending to each address.
for i, variant in enumerate(IMPORT_VARIANTS): for i, variant in enumerate(IMPORT_VARIANTS):
variant.sent_amount = 10 - (2 * i + 1) / 8.0 variant.sent_amount = 1 - (2 * i + 1) / 128
variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount) variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount)
# Generate a block containing the new transactions. # Generate a block containing the new transactions.

View file

@ -3,6 +3,8 @@
# Distributed under the MIT software license, see the accompanying # Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php. # file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test the importmulti RPC.""" """Test the importmulti RPC."""
from test_framework import script
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error
@ -79,16 +81,17 @@ class ImportMultiTest (BitcoinTestFramework):
assert_equal(address_assert['ismine'], False) assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp) assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + !internal # Nonstandard scriptPubKey + !internal
self.log.info("Should not import a scriptPubKey without internal flag") self.log.info("Should not import a nonstandard scriptPubKey without internal flag")
nonstandardScriptPubKey = address['scriptPubKey'] + bytes_to_hex_str(script.CScript([script.OP_NOP]))
address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
result = self.nodes[1].importmulti([{ result = self.nodes[1].importmulti([{
"scriptPubKey": address['scriptPubKey'], "scriptPubKey": nonstandardScriptPubKey,
"timestamp": "now", "timestamp": "now",
}]) }])
assert_equal(result[0]['success'], False) assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -8) assert_equal(result[0]['error']['code'], -8)
assert_equal(result[0]['error']['message'], 'Internal must be set for hex scriptPubKey') assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.')
address_assert = self.nodes[1].getaddressinfo(address['address']) address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False) assert_equal(address_assert['ismine'], False)
@ -128,18 +131,18 @@ class ImportMultiTest (BitcoinTestFramework):
assert_equal(address_assert['ismine'], False) assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp) assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + Public key + !internal # Nonstandard scriptPubKey + Public key + !internal
self.log.info("Should not import a scriptPubKey without internal and with public key") self.log.info("Should not import a nonstandard scriptPubKey without internal and with public key")
address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
request = [{ request = [{
"scriptPubKey": address['scriptPubKey'], "scriptPubKey": nonstandardScriptPubKey,
"timestamp": "now", "timestamp": "now",
"pubkeys": [ address['pubkey'] ] "pubkeys": [ address['pubkey'] ]
}] }]
result = self.nodes[1].importmulti(request) result = self.nodes[1].importmulti(request)
assert_equal(result[0]['success'], False) assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -8) assert_equal(result[0]['error']['code'], -8)
assert_equal(result[0]['error']['message'], 'Internal must be set for hex scriptPubKey') assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.')
address_assert = self.nodes[1].getaddressinfo(address['address']) address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False) assert_equal(address_assert['ismine'], False)
@ -207,17 +210,17 @@ class ImportMultiTest (BitcoinTestFramework):
assert_equal(address_assert['ismine'], True) assert_equal(address_assert['ismine'], True)
assert_equal(address_assert['timestamp'], timestamp) assert_equal(address_assert['timestamp'], timestamp)
# ScriptPubKey + Private key + !internal # Nonstandard scriptPubKey + Private key + !internal
self.log.info("Should not import a scriptPubKey without internal and with private key") self.log.info("Should not import a nonstandard scriptPubKey without internal and with private key")
address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
result = self.nodes[1].importmulti([{ result = self.nodes[1].importmulti([{
"scriptPubKey": address['scriptPubKey'], "scriptPubKey": nonstandardScriptPubKey,
"timestamp": "now", "timestamp": "now",
"keys": [ self.nodes[0].dumpprivkey(address['address']) ] "keys": [ self.nodes[0].dumpprivkey(address['address']) ]
}]) }])
assert_equal(result[0]['success'], False) assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -8) assert_equal(result[0]['error']['code'], -8)
assert_equal(result[0]['error']['message'], 'Internal must be set for hex scriptPubKey') assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.')
address_assert = self.nodes[1].getaddressinfo(address['address']) address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False) assert_equal(address_assert['ismine'], False)