partial Merge #14454: Add SegWit support to importmulti (#4440)

* partial Merge #14454: Add SegWit support to importmulti

c11875c5908a17314bb38caa911507dc6401ec49 Add segwit address tests for importmulti (MeshCollider)
201451b1ca3c6db3b13f9491a81db5b120b864bb Make getaddressinfo return solvability (MeshCollider)
1753d217ead7e2de35b3df6cd6573a1c9a068f84 Add release notes for importmulti segwit change (MeshCollider)
353c064596fc2e2c149987ac3b3c11b4c90c4d5f Fix typo in test_framework/blocktools (MeshCollider)
f6ed748cf045d7f0d9a49e15cc0c0001610b9231 Add SegWit support to importmulti with some ProcessImport cleanup (MeshCollider)

Pull request description:

  Add support for segwit to importmulti, supports P2WSH, P2WPKH, P2SH-P2WPKH, P2SH-P2WSH. Adds a new `witnessscript` parameter which must be used for the witness scripts in the relevant situations.

  Also includes some tests for the various import types.

  ~Also makes the change in #14019 redundant, but cherry-picks the test from that PR to test the behavior (@achow101).~

  Fixes #12253, also addresses the second point in #12703, and fixes #14407

Tree-SHA512: 775a755c524d1c387a99acddd772f677d2073876b72403dcfb92c59f9b405ae13ceedcf4dbd2ee1d7a8db91c494f67ca137161032ee3a2071282eeb411be090a

# Conflicts:
#	src/wallet/rpcdump.cpp
#	test/functional/test_framework/blocktools.py
#	test/functional/wallet_importmulti.py

* make linter happy

Signed-off-by: pasta <pasta@dashboost.org>

* Fixes: trivial + linter + add missing consistency check + more/redo 14679

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
This commit is contained in:
PastaPastaPasta 2021-09-28 15:48:33 -04:00 committed by GitHub
parent d2c975dbd7
commit 908b6e6535
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 120 additions and 181 deletions

View File

@ -1032,15 +1032,13 @@ UniValue dumpwallet(const JSONRPCRequest& request)
static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
{
try {
bool success = false;
// Required fields.
// First ensure scriptPubKey has either a script or JSON with "address" string
const UniValue& scriptPubKey = data["scriptPubKey"];
// Should have script or JSON with "address".
if (!(scriptPubKey.getType() == UniValue::VOBJ && scriptPubKey.exists("address")) && !(scriptPubKey.getType() == UniValue::VSTR)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid scriptPubKey");
bool isScript = scriptPubKey.getType() == UniValue::VSTR;
if (!isScript && !(scriptPubKey.getType() == UniValue::VOBJ && scriptPubKey.exists("address"))) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "scriptPubKey must be string with script or JSON with address string");
}
const std::string& output = isScript ? scriptPubKey.get_str() : scriptPubKey["address"].get_str();
// Optional fields.
const std::string& strRedeemScript = data.exists("redeemscript") ? data["redeemscript"].get_str() : "";
@ -1048,11 +1046,7 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
const UniValue& keys = data.exists("keys") ? data["keys"].get_array() : UniValue();
const bool internal = data.exists("internal") ? data["internal"].get_bool() : false;
const bool watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false;
const std::string& label = data.exists("label") && !internal ? data["label"].get_str() : "";
bool isScript = scriptPubKey.getType() == UniValue::VSTR;
bool isP2SH = strRedeemScript.length() > 0;
const std::string& output = isScript ? scriptPubKey.get_str() : scriptPubKey["address"].get_str();
const std::string& label = data.exists("label") ? data["label"].get_str() : "";
// Parse the output.
// If private keys are disabled, abort if private keys are being imported
@ -1084,35 +1078,32 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
// Watchonly and private keys
if (watchOnly && keys.size()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between watchonly and keys");
throw JSONRPCError(RPC_INVALID_PARAMETER, "Watch-only addresses should not include private keys");
}
// Internal + Label
// Internal addresses should not have a label
if (internal && data.exists("label")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between internal and label");
throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label");
}
// 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");
}
// Invalid P2SH redeemScript
if (isP2SH && !IsHex(strRedeemScript)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script");
}
// Process. //
CScript scriptpubkey_script = script;
CTxDestination scriptpubkey_dest = dest;
// P2SH
if (isP2SH) {
if (!strRedeemScript.empty() && script.IsPayToScriptHash()) {
// Check the redeemScript is valid
if (!IsHex(strRedeemScript)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script: must be hex string");
}
// Import redeem script.
std::vector<unsigned char> vData(ParseHex(strRedeemScript));
CScript redeemScript = CScript(vData.begin(), vData.end());
CScriptID redeem_id(redeemScript);
// Invalid P2SH address
if (!script.IsPayToScriptHash()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid P2SH address / script");
// Check that the redeemScript and scriptPubKey match
if (GetScriptForDestination(redeem_id) != script) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The redeemScript does not match the scriptPubKey");
}
pwallet->MarkDirty();
@ -1121,103 +1112,49 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
CScriptID redeem_id(redeemScript);
if (!pwallet->HaveCScript(redeem_id) && !pwallet->AddCScript(redeemScript)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet");
}
CScript redeemDestination = GetScriptForDestination(redeem_id);
if (::IsMine(*pwallet, redeemDestination) == ISMINE_SPENDABLE) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
// Now set script to the redeemScript so we parse the inner script as P2WSH or P2WPKH below
script = redeemScript;
ExtractDestination(script, dest);
}
pwallet->MarkDirty();
if (!pwallet->AddWatchOnly(redeemDestination, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
// (P2SH-)P2PK/P2PKH
if (dest.type() == typeid(CKeyID)) {
if (keys.size() > 1 || pubKeys.size() > 1) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "More than one key given for one single-key address");
}
// add to address book or update label
if (IsValidDestination(dest)) {
pwallet->SetAddressBook(dest, label, "receive");
}
// Import private keys.
CPubKey pubkey;
if (keys.size()) {
for (size_t i = 0; i < keys.size(); i++) {
const std::string& privkey = keys[i].get_str();
CKey key = DecodeSecret(privkey);
if (!key.IsValid()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
pubkey = DecodeSecret(keys[0].get_str()).GetPubKey();
}
CPubKey pubkey = key.GetPubKey();
assert(key.VerifyPubKey(pubkey));
CKeyID vchAddress = pubkey.GetID();
pwallet->MarkDirty();
pwallet->SetAddressBook(vchAddress, label, "receive");
if (pwallet->HaveKey(vchAddress)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Already have this key");
}
pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp;
if (!pwallet->AddKeyPubKey(key, pubkey)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
}
pwallet->UpdateTimeFirstKey(timestamp);
}
}
success = true;
} else {
// Import public keys.
if (pubKeys.size() && keys.size() == 0) {
if (pubKeys.size()) {
const std::string& strPubKey = pubKeys[0].get_str();
if (!IsHex(strPubKey)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string");
}
std::vector<unsigned char> vData(ParseHex(strPubKey));
CPubKey pubKey(vData.begin(), vData.end());
if (!pubKey.IsFullyValid()) {
std::vector<unsigned char> vData(ParseHex(pubKeys[0].get_str()));
CPubKey pubkey_temp(vData.begin(), vData.end());
if (pubkey.size() && pubkey_temp != pubkey) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key does not match public key for address");
}
pubkey = pubkey_temp;
}
if (pubkey.size() > 0) {
if (!pubkey.IsFullyValid()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key");
}
CTxDestination pubkey_dest = pubKey.GetID();
// Consistency check.
// Check the key corresponds to the destination given
CTxDestination pubkey_dest = pubkey.GetID();
if (!(pubkey_dest == dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Key does not match address destination");
}
CScript pubKeyScript = GetScriptForDestination(pubkey_dest);
if (::IsMine(*pwallet, pubKeyScript) == ISMINE_SPENDABLE) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
}
pwallet->MarkDirty();
if (!pwallet->AddWatchOnly(pubKeyScript, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
// add to address book or update label
if (IsValidDestination(pubkey_dest)) {
pwallet->SetAddressBook(pubkey_dest, label, "receive");
}
// TODO Is this necessary?
CScript scriptRawPubKey = GetScriptForRawPubKey(pubKey);
// This is necessary to force the wallet to import the pubKey
CScript scriptRawPubKey = GetScriptForRawPubKey(pubkey);
if (::IsMine(*pwallet, scriptRawPubKey) == ISMINE_SPENDABLE) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
@ -1228,13 +1165,33 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
if (!pwallet->AddWatchOnly(scriptRawPubKey, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
}
}
success = true;
// Import the address
if (::IsMine(*pwallet, scriptpubkey_script) == ISMINE_SPENDABLE) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
}
pwallet->MarkDirty();
if (!pwallet->AddWatchOnly(scriptpubkey_script, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
if (!watchOnly && !pwallet->HaveCScript(CScriptID(scriptpubkey_script)) && !pwallet->AddCScript(scriptpubkey_script)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding scriptPubKey script to wallet");
}
// if not internal add to address book or update label
if (!internal) {
assert(IsValidDestination(scriptpubkey_dest));
pwallet->SetAddressBook(scriptpubkey_dest, label, "receive");
}
// Import private keys.
if (keys.size()) {
const std::string& strPrivkey = keys[0].get_str();
for (size_t i = 0; i < keys.size(); i++) {
const std::string& strPrivkey = keys[i].get_str();
// Checks.
CKey key = DecodeSecret(strPrivkey);
@ -1246,19 +1203,11 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
CPubKey pubKey = key.GetPubKey();
assert(key.VerifyPubKey(pubKey));
CTxDestination pubkey_dest = pubKey.GetID();
// Consistency check.
if (!(pubkey_dest == dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
}
CKeyID vchAddress = pubKey.GetID();
pwallet->MarkDirty();
pwallet->SetAddressBook(vchAddress, label, "receive");
if (pwallet->HaveKey(vchAddress)) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Already have this key");
}
pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp;
@ -1268,34 +1217,10 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
}
pwallet->UpdateTimeFirstKey(timestamp);
success = true;
}
// Import scriptPubKey only.
if (pubKeys.size() == 0 && keys.size() == 0) {
if (::IsMine(*pwallet, script) == ISMINE_SPENDABLE) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
}
pwallet->MarkDirty();
if (!pwallet->AddWatchOnly(script, timestamp)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
// if not internal add to address book or update label
if (!internal) {
assert(IsValidDestination(dest));
pwallet->SetAddressBook(dest, label, "receive");
}
success = true;
}
}
UniValue result = UniValue(UniValue::VOBJ);
result.pushKV("success", UniValue(success));
result.pushKV("success", UniValue(true));
return result;
} catch (const UniValue& e) {
UniValue result = UniValue(UniValue::VOBJ);

View File

@ -3650,6 +3650,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
" \"address\" : \"address\", (string) The dash address validated\n"
" \"scriptPubKey\" : \"hex\", (string) The hex-encoded scriptPubKey generated by the address\n"
" \"ismine\" : true|false, (boolean) If the address is yours or not\n"
" \"solvable\" : true|false, (boolean) If the address is solvable by the wallet\n"
" \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n"
" \"isscript\" : true|false, (boolean) If the key is a script\n"
" \"ischange\" : true|false, (boolean) If the address was used for change output\n"
@ -3701,6 +3702,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
isminetype mine = IsMine(*pwallet, dest);
ret.pushKV("ismine", bool(mine & ISMINE_SPENDABLE));
ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY));
ret.pushKV("solvable", IsSolvable(*pwallet, scriptPubKey));
UniValue detail = DescribeWalletAddress(pwallet, dest);
ret.pushKVs(detail);
if (pwallet->mapAddressBook.count(dest)) {

View File

@ -1,5 +1,5 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2016 The Bitcoin Core developers
# Copyright (c) 2014-2018 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test the importmulti RPC."""
@ -12,7 +12,6 @@ from test_framework.util import (
assert_raises_rpc_error,
)
class ImportMultiTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
@ -90,6 +89,19 @@ class ImportMultiTest(BitcoinTestFramework):
assert_equal(address_assert['timestamp'], timestamp)
assert_equal(address_assert['ischange'], True)
# ScriptPubKey + internal + label
self.log.info("Should not allow a label to be specified when internal is true")
address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
result = self.nodes[1].importmulti([{
"scriptPubKey": address['scriptPubKey'],
"timestamp": "now",
"internal": True,
"label": "Example label"
}])
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -8)
assert_equal(result[0]['error']['message'], 'Internal addresses should not have a label')
# Nonstandard scriptPubKey + !internal
self.log.info("Should not import a nonstandard scriptPubKey without internal flag")
nonstandardScriptPubKey = address['scriptPubKey'] + script.CScript([script.OP_NOP]).hex()
@ -199,7 +211,7 @@ class ImportMultiTest(BitcoinTestFramework):
}])
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -8)
assert_equal(result[0]['error']['message'], 'Incompatibility found between watchonly and keys')
assert_equal(result[0]['error']['message'], 'Watch-only addresses should not include private keys')
address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
@ -340,7 +352,7 @@ class ImportMultiTest(BitcoinTestFramework):
}])
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -8)
assert_equal(result[0]['error']['message'], 'Incompatibility found between watchonly and keys')
assert_equal(result[0]['error']['message'], 'Watch-only addresses should not include private keys')
# Address + Public key + !Internal + Wrong pubkey
@ -356,7 +368,7 @@ class ImportMultiTest(BitcoinTestFramework):
}])
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -5)
assert_equal(result[0]['error']['message'], 'Consistency check failed')
assert_equal(result[0]['error']['message'], 'Key does not match address destination')
address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
@ -376,7 +388,7 @@ class ImportMultiTest(BitcoinTestFramework):
result = self.nodes[1].importmulti(request)
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -5)
assert_equal(result[0]['error']['message'], 'Consistency check failed')
assert_equal(result[0]['error']['message'], 'Key does not match address destination')
address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
@ -396,7 +408,7 @@ class ImportMultiTest(BitcoinTestFramework):
}])
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -5)
assert_equal(result[0]['error']['message'], 'Consistency check failed')
assert_equal(result[0]['error']['message'], 'Key does not match address destination')
address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)
@ -415,7 +427,7 @@ class ImportMultiTest(BitcoinTestFramework):
}])
assert_equal(result[0]['success'], False)
assert_equal(result[0]['error']['code'], -5)
assert_equal(result[0]['error']['message'], 'Consistency check failed')
assert_equal(result[0]['error']['message'], 'Key does not match address destination')
address_assert = self.nodes[1].getaddressinfo(address['address'])
assert_equal(address_assert['iswatchonly'], False)
assert_equal(address_assert['ismine'], False)