Skip to content

Commit

Permalink
Merge bitcoin#9332: Let wallet importmulti RPC accept labels for stan…
Browse files Browse the repository at this point in the history 
…dard scriptPubKeys

98ea64c 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
  • Loading branch information
@laanwj @PastaPastaPasta
laanwj authored and PastaPastaPasta committed Jun 28, 2021
1 parent c759dbf commit 8ea4c51
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 51 deletions.
42 changes: 8 additions & 34 deletions src/wallet/rpcdump.cpp
View file
Expand Up @@ -1049,6 +1049,9 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con

std::vector<unsigned char> vData(ParseHex(output));
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
Expand All @@ -1061,11 +1064,6 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
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.
if (!isP2SH && (keys.size() > 1 || pubKeys.size() > 1)) { // Address / scriptPubKey
throw JSONRPCError(RPC_INVALID_PARAMETER, "More than private key given for one address");
Expand Down Expand Up @@ -1169,21 +1167,10 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
CTxDestination pubkey_dest = pubKey.GetID();

// Consistency check.
if (!isScript && !(pubkey_dest == dest)) {
if (!(pubkey_dest == dest)) {
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);

if (::IsMine(*pwallet, pubKeyScript) == ISMINE_SPENDABLE) {
Expand Down Expand Up @@ -1234,21 +1221,10 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
CTxDestination pubkey_dest = pubKey.GetID();

// Consistency check.
if (!isScript && !(pubkey_dest == dest)) {
if (!(pubkey_dest == dest)) {
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();
pwallet->MarkDirty();
pwallet->SetAddressBook(vchAddress, label, "receive");
Expand Down Expand Up @@ -1280,11 +1256,9 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}

if (scriptPubKey.getType() == UniValue::VOBJ) {
// add to address book or update label
if (IsValidDestination(dest)) {
pwallet->SetAddressBook(dest, label, "receive");
}
// add to address book or update label
if (IsValidDestination(dest)) {
pwallet->SetAddressBook(dest, label, "receive");
}

success = true;
Expand Down
10 changes: 5 additions & 5 deletions test/functional/wallet_import_rescan.py
View file
Expand Up @@ -27,7 +27,7 @@
import itertools
import sys

Call = enum.Enum("Call", "single multi")
Call = enum.Enum("Call", "single multiaddress multiscript")
Data = enum.Enum("Data", "address pub priv")
Rescan = enum.Enum("Rescan", "no yes late_timestamp")

Expand All @@ -54,11 +54,11 @@ def do_import(self, timestamp):
response = self.try_rpc(self.node.importprivkey, privkey=self.key, label=self.label, rescan=rescan)
assert_equal(response, None)

elif self.call == Call.multi:
elif self.call in (Call.multiaddress, Call.multiscript):
response = self.node.importmulti([{
"scriptPubKey": {
"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),
"pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [],
"keys": [self.key] if self.data == Data.priv else [],
Expand Down Expand Up @@ -143,7 +143,7 @@ def run_test(self):
variant.label = "label {} {}".format(i, variant)
variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress(variant.label))
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)

# Generate a block containing the initial transactions, then another
Expand Down Expand Up @@ -173,7 +173,7 @@ def run_test(self):

# Create new transactions sending to each address.
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)

# Generate a block containing the new transactions.
Expand Down
27 changes: 15 additions & 12 deletions test/functional/wallet_importmulti.py
View file
Expand Up @@ -3,6 +3,8 @@
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test the importmulti RPC."""

from test_framework import script
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import *

Expand Down Expand Up @@ -78,16 +80,17 @@ def run_test (self):
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp)

# ScriptPubKey + !internal
self.log.info("Should not import a scriptPubKey without internal flag")
# Nonstandard scriptPubKey + !internal
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())
result = self.nodes[1].importmulti([{
"scriptPubKey": address['scriptPubKey'],
"scriptPubKey": nonstandardScriptPubKey,
"timestamp": "now",
}])
assert_equal(result[0]['success'], False)
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'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
Expand Down Expand Up @@ -127,18 +130,18 @@ def run_test (self):
assert_equal(address_assert['ismine'], False)
assert_equal(address_assert['timestamp'], timestamp)

# ScriptPubKey + Public key + !internal
self.log.info("Should not import a scriptPubKey without internal and with public key")
# Nonstandard scriptPubKey + Public key + !internal
self.log.info("Should not import a nonstandard scriptPubKey without internal and with public key")
address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
request = [{
"scriptPubKey": address['scriptPubKey'],
"scriptPubKey": nonstandardScriptPubKey,
"timestamp": "now",
"pubkeys": [ address['pubkey'] ]
}]
result = self.nodes[1].importmulti(request)
assert_equal(result[0]['success'], False)
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'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
Expand Down Expand Up @@ -206,17 +209,17 @@ def run_test (self):
assert_equal(address_assert['ismine'], True)
assert_equal(address_assert['timestamp'], timestamp)

# ScriptPubKey + Private key + !internal
self.log.info("Should not import a scriptPubKey without internal and with private key")
# Nonstandard scriptPubKey + Private key + !internal
self.log.info("Should not import a nonstandard scriptPubKey without internal and with private key")
address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
result = self.nodes[1].importmulti([{
"scriptPubKey": address['scriptPubKey'],
"scriptPubKey": nonstandardScriptPubKey,
"timestamp": "now",
"keys": [ self.nodes[0].dumpprivkey(address['address']) ]
}])
assert_equal(result[0]['success'], False)
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'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
Expand Down

0 comments on commit 8ea4c51

Please sign in to comment.