From 0c8ea6380c9f402ed9777fd015b117ba13125a35 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:48:31 -0800 Subject: [PATCH 01/12] Abstract out IsSolvable from Witnessifier --- src/policy/policy.h | 36 ++++++++++++++++++------------------ src/script/sign.cpp | 19 +++++++++++++++++++ src/script/sign.h | 6 ++++++ src/wallet/rpcwallet.cpp | 14 ++------------ 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/policy/policy.h b/src/policy/policy.h index f3f8ebbbb4c40..16a1bc67db5e5 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -49,28 +49,28 @@ static const unsigned int DUST_RELAY_TX_FEE = 3000; * with. However scripts violating these flags may still be present in valid * blocks and we must accept those blocks. */ -static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS | - SCRIPT_VERIFY_DERSIG | - SCRIPT_VERIFY_STRICTENC | - SCRIPT_VERIFY_MINIMALDATA | - SCRIPT_VERIFY_NULLDUMMY | - SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS | - SCRIPT_VERIFY_CLEANSTACK | - SCRIPT_VERIFY_MINIMALIF | - SCRIPT_VERIFY_NULLFAIL | - SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY | - SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | - SCRIPT_VERIFY_LOW_S | - SCRIPT_VERIFY_WITNESS | - SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM | - SCRIPT_VERIFY_WITNESS_PUBKEYTYPE; +static constexpr unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS | + SCRIPT_VERIFY_DERSIG | + SCRIPT_VERIFY_STRICTENC | + SCRIPT_VERIFY_MINIMALDATA | + SCRIPT_VERIFY_NULLDUMMY | + SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS | + SCRIPT_VERIFY_CLEANSTACK | + SCRIPT_VERIFY_MINIMALIF | + SCRIPT_VERIFY_NULLFAIL | + SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY | + SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | + SCRIPT_VERIFY_LOW_S | + SCRIPT_VERIFY_WITNESS | + SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM | + SCRIPT_VERIFY_WITNESS_PUBKEYTYPE; /** For convenience, standard but not mandatory verify flags. */ -static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS; +static constexpr unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS; /** Used as the flags parameter to sequence and nLocktime checks in non-consensus code. */ -static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_VERIFY_SEQUENCE | - LOCKTIME_MEDIAN_TIME_PAST; +static constexpr unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_VERIFY_SEQUENCE | + LOCKTIME_MEDIAN_TIME_PAST; CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 117a4d8a52aa0..49099bf7b0a44 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -422,3 +422,22 @@ bool DummySignatureCreator::CreateSig(std::vector& vchSig, const vchSig[6 + 33 + 32] = SIGHASH_ALL; return true; } + +bool IsSolvable(const CKeyStore& store, const CScript& script) +{ + // This check is to make sure that the script we created can actually be solved for and signed by us + // if we were to have the private keys. This is just to make sure that the script is valid and that, + // if found in a transaction, we would still accept and relay that transaction. In particular, + // it will reject witness outputs that require signing with an uncompressed public key. + DummySignatureCreator creator(&store); + SignatureData sigs; + // Make sure that STANDARD_SCRIPT_VERIFY_FLAGS includes SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, the most + // important property this function is designed to test for. + static_assert(STANDARD_SCRIPT_VERIFY_FLAGS & SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, "IsSolvable requires standard script flags to include WITNESS_PUBKEYTYPE"); + if (ProduceSignature(creator, script, sigs)) { + // VerifyScript check is just defensive, and should never fail. + assert(VerifyScript(sigs.scriptSig, script, &sigs.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker())); + return true; + } + return false; +} diff --git a/src/script/sign.h b/src/script/sign.h index 400c0c08658b4..ef6aa7f311b0d 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -81,4 +81,10 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn); void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data); +/* Check whether we know how to sign for an output like this, assuming we + * have all private keys. While this function does not need private keys, the passed + * keystore is used to look up public keys and redeemscripts by hash. + * Solvability is unrelated to whether we consider this output to be ours. */ +bool IsSolvable(const CKeyStore& store, const CScript& script); + #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 473acd8367d4a..760777ca29712 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1204,12 +1204,7 @@ class Witnessifier : public boost::static_visitor if (pwallet) { CScript basescript = GetScriptForDestination(keyID); CScript witscript = GetScriptForWitness(basescript); - SignatureData sigs; - // This check is to make sure that the script we created can actually be solved for and signed by us - // if we were to have the private keys. This is just to make sure that the script is valid and that, - // if found in a transaction, we would still accept and relay that transaction. - if (!ProduceSignature(DummySignatureCreator(pwallet), witscript, sigs) || - !VerifyScript(sigs.scriptSig, witscript, &sigs.scriptWitness, MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, DummySignatureCreator(pwallet).Checker())) { + if (!IsSolvable(*pwallet, witscript)) { return false; } return ExtractDestination(witscript, result); @@ -1228,12 +1223,7 @@ class Witnessifier : public boost::static_visitor return true; } CScript witscript = GetScriptForWitness(subscript); - SignatureData sigs; - // This check is to make sure that the script we created can actually be solved for and signed by us - // if we were to have the private keys. This is just to make sure that the script is valid and that, - // if found in a transaction, we would still accept and relay that transaction. - if (!ProduceSignature(DummySignatureCreator(pwallet), witscript, sigs) || - !VerifyScript(sigs.scriptSig, witscript, &sigs.scriptWitness, MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, DummySignatureCreator(pwallet).Checker())) { + if (!IsSolvable(*pwallet, witscript)) { return false; } return ExtractDestination(witscript, result); From cbe197470ecc1f7b48771c4e7b654ab030af4c9e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 17 Dec 2017 20:34:39 -0800 Subject: [PATCH 02/12] [refactor] GetAccount{PubKey,Address} -> GetAccountDestination --- src/wallet/rpcwallet.cpp | 14 +++++++------- src/wallet/wallet.cpp | 9 +++++---- src/wallet/wallet.h | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 760777ca29712..b366992dec2d0 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -175,14 +175,14 @@ UniValue getnewaddress(const JSONRPCRequest& request) } -CTxDestination GetAccountAddress(CWallet* const pwallet, std::string strAccount, bool bForceNew=false) +CTxDestination GetAccountDestination(CWallet* const pwallet, std::string strAccount, bool bForceNew=false) { - CPubKey pubKey; - if (!pwallet->GetAccountPubkey(pubKey, strAccount, bForceNew)) { + CTxDestination dest; + if (!pwallet->GetAccountDestination(dest, strAccount, bForceNew)) { throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); } - return pubKey.GetID(); + return dest; } UniValue getaccountaddress(const JSONRPCRequest& request) @@ -214,7 +214,7 @@ UniValue getaccountaddress(const JSONRPCRequest& request) UniValue ret(UniValue::VSTR); - ret = EncodeDestination(GetAccountAddress(pwallet, strAccount)); + ret = EncodeDestination(GetAccountDestination(pwallet, strAccount)); return ret; } @@ -292,8 +292,8 @@ UniValue setaccount(const JSONRPCRequest& request) // Detect when changing the account of an address that is the 'unused current key' of another account: if (pwallet->mapAddressBook.count(dest)) { std::string strOldAccount = pwallet->mapAddressBook[dest].name; - if (dest == GetAccountAddress(pwallet, strOldAccount)) { - GetAccountAddress(pwallet, strOldAccount, true); + if (dest == GetAccountDestination(pwallet, strOldAccount)) { + GetAccountDestination(pwallet, strOldAccount, true); } } pwallet->SetAddressBook(dest, strAccount, "receive"); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 873a8d855e570..f73f787259a31 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -821,7 +821,7 @@ bool CWallet::AccountMove(std::string strFrom, std::string strTo, CAmount nAmoun return true; } -bool CWallet::GetAccountPubkey(CPubKey &pubKey, std::string strAccount, bool bForceNew) +bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount, bool bForceNew) { CWalletDB walletdb(*dbw); @@ -850,12 +850,13 @@ bool CWallet::GetAccountPubkey(CPubKey &pubKey, std::string strAccount, bool bFo if (!GetKeyFromPool(account.vchPubKey, false)) return false; - SetAddressBook(account.vchPubKey.GetID(), strAccount, "receive"); + dest = account.vchPubKey.GetID(); + SetAddressBook(dest, strAccount, "receive"); walletdb.WriteAccount(strAccount, account); + } else { + dest = account.vchPubKey.GetID(); } - pubKey = account.vchPubKey; - return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 93d1857c7fbea..e64ea2534afec 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -923,7 +923,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface int64_t IncOrderPosNext(CWalletDB *pwalletdb = nullptr); DBErrors ReorderTransactions(); bool AccountMove(std::string strFrom, std::string strTo, CAmount nAmount, std::string strComment = ""); - bool GetAccountPubkey(CPubKey &pubKey, std::string strAccount, bool bForceNew = false); + bool GetAccountDestination(CTxDestination &dest, std::string strAccount, bool bForceNew = false); void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); From 985c79552ceb6a5f5812d421dc5c86fa3b1cc41d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:48:38 -0800 Subject: [PATCH 03/12] Improve witness destination types and use them more --- src/script/standard.cpp | 11 +++-------- src/script/standard.h | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/script/standard.cpp b/src/script/standard.cpp index b7b33fade6c35..0d9f6ae1f9f3b 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -348,19 +348,14 @@ CScript GetScriptForWitness(const CScript& redeemscript) std::vector > vSolutions; if (Solver(redeemscript, typ, vSolutions)) { if (typ == TX_PUBKEY) { - unsigned char h160[20]; - CHash160().Write(&vSolutions[0][0], vSolutions[0].size()).Finalize(h160); - ret << OP_0 << std::vector(&h160[0], &h160[20]); - return ret; + return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0].begin(), vSolutions[0].end()))); } else if (typ == TX_PUBKEYHASH) { - ret << OP_0 << vSolutions[0]; - return ret; + return GetScriptForDestination(WitnessV0KeyHash(vSolutions[0])); } } uint256 hash; CSHA256().Write(&redeemscript[0], redeemscript.size()).Finalize(hash.begin()); - ret << OP_0 << ToByteVector(hash); - return ret; + return GetScriptForDestination(WitnessV0ScriptHash(hash)); } bool IsValidDestination(const CTxDestination& dest) { diff --git a/src/script/standard.h b/src/script/standard.h index 3eeeabdc159bd..b1c314fa72c5d 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -73,8 +73,19 @@ class CNoDestination { friend bool operator<(const CNoDestination &a, const CNoDestination &b) { return true; } }; -struct WitnessV0ScriptHash : public uint256 {}; -struct WitnessV0KeyHash : public uint160 {}; +struct WitnessV0ScriptHash : public uint256 +{ + WitnessV0ScriptHash() : uint256() {} + explicit WitnessV0ScriptHash(const uint256& hash) : uint256(hash) {} + using uint256::uint256; +}; + +struct WitnessV0KeyHash : public uint160 +{ + WitnessV0KeyHash() : uint160() {} + explicit WitnessV0KeyHash(const uint160& hash) : uint160(hash) {} + using uint160::uint160; +}; //! CTxDestination subtype to encode any future Witness version struct WitnessUnknown @@ -164,6 +175,9 @@ CScript GetScriptForMultisig(int nRequired, const std::vector& keys); * Generate a pay-to-witness script for the given redeem script. If the redeem * script is P2PK or P2PKH, this returns a P2WPKH script, otherwise it returns a * P2WSH script. + * + * TODO: replace calls to GetScriptForWitness with GetScriptForDestination using + * the various witness-specific CTxDestination subtypes. */ CScript GetScriptForWitness(const CScript& redeemscript); From 30a27dc5b18a3ab82a9768710b24d4e7e9661658 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:48:41 -0800 Subject: [PATCH 04/12] Expose method to find key for a single-key destination --- src/keystore.cpp | 22 ++++++++++++++++++++++ src/keystore.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/src/keystore.cpp b/src/keystore.cpp index 4ab089e032759..cbcc6d7ca9a88 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -136,3 +136,25 @@ bool CBasicKeyStore::HaveWatchOnly() const LOCK(cs_KeyStore); return (!setWatchOnly.empty()); } + +CKeyID GetKeyForDestination(const CKeyStore& store, const CTxDestination& dest) +{ + // Only supports destinations which map to single public keys, i.e. P2PKH, + // P2WPKH, and P2SH-P2WPKH. + if (auto id = boost::get(&dest)) { + return *id; + } + if (auto witness_id = boost::get(&dest)) { + return CKeyID(*witness_id); + } + if (auto script_id = boost::get(&dest)) { + CScript script; + CTxDestination inner_dest; + if (store.GetCScript(*script_id, script) && ExtractDestination(script, inner_dest)) { + if (auto inner_witness_id = boost::get(&inner_dest)) { + return CKeyID(*inner_witness_id); + } + } + } + return CKeyID(); +} diff --git a/src/keystore.h b/src/keystore.h index 4e6d8e8a27631..0f3b5706f21b3 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -78,4 +78,7 @@ class CBasicKeyStore : public CKeyStore typedef std::vector > CKeyingMaterial; typedef std::map > > CryptedKeyMap; +/** Return the CKeyID of the key involved in a script (if there is a unique one). */ +CKeyID GetKeyForDestination(const CKeyStore& store, const CTxDestination& dest); + #endif // BITCOIN_KEYSTORE_H From 3eaa003c888f80207e8ff132f78417ff373ddfa3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:48:45 -0800 Subject: [PATCH 05/12] Extend validateaddress information for P2SH-embedded witness This adds new fields 'pubkeys' and 'embedded' to the RPC's output, and improves the documentation for previously added 'witness_version' and 'witness_program' fields. --- src/rpc/misc.cpp | 86 +++++++++++++++++++++++++++++++------------ src/script/standard.h | 4 ++ 2 files changed, 66 insertions(+), 24 deletions(-) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 327af2e237347..f36879f3b188e 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -40,6 +40,46 @@ class DescribeAddressVisitor : public boost::static_visitor explicit DescribeAddressVisitor(CWallet *_pwallet) : pwallet(_pwallet) {} + void ProcessSubScript(const CScript& subscript, UniValue& obj, bool include_addresses = false) const + { + // Always present: script type and redeemscript + txnouttype which_type; + std::vector> solutions_data; + Solver(subscript, which_type, solutions_data); + obj.pushKV("script", GetTxnOutputType(which_type)); + obj.pushKV("hex", HexStr(subscript.begin(), subscript.end())); + + CTxDestination embedded; + UniValue a(UniValue::VARR); + if (ExtractDestination(subscript, embedded)) { + // Only when the script corresponds to an address. + UniValue subobj = boost::apply_visitor(*this, embedded); + subobj.pushKV("address", EncodeDestination(embedded)); + subobj.pushKV("scriptPubKey", HexStr(subscript.begin(), subscript.end())); + // Always report the pubkey at the top level, so that `getnewaddress()['pubkey']` always works. + if (subobj.exists("pubkey")) obj.pushKV("pubkey", subobj["pubkey"]); + obj.pushKV("embedded", std::move(subobj)); + if (include_addresses) a.push_back(EncodeDestination(embedded)); + } else if (which_type == TX_MULTISIG) { + // Also report some information on multisig scripts (which do not have a corresponding address). + // TODO: abstract out the common functionality between this logic and ExtractDestinations. + obj.pushKV("sigsrequired", solutions_data[0][0]); + UniValue pubkeys(UniValue::VARR); + for (size_t i = 1; i < solutions_data.size() - 1; ++i) { + CPubKey key(solutions_data[i].begin(), solutions_data[i].end()); + if (include_addresses) a.push_back(EncodeDestination(key.GetID())); + pubkeys.push_back(HexStr(key.begin(), key.end())); + } + obj.pushKV("pubkeys", std::move(pubkeys)); + } + + // The "addresses" field is confusing because it refers to public keys using their P2PKH address. + // For that reason, only add the 'addresses' field when needed for backward compatibility. New applications + // can use the 'embedded'->'address' field for P2SH or P2WSH wrapped addresses, and 'pubkeys' for + // inspecting multisig participants. + if (include_addresses) obj.pushKV("addresses", std::move(a)); + } + UniValue operator()(const CNoDestination &dest) const { return UniValue(UniValue::VOBJ); } UniValue operator()(const CKeyID &keyID) const { @@ -60,19 +100,7 @@ class DescribeAddressVisitor : public boost::static_visitor obj.push_back(Pair("isscript", true)); obj.push_back(Pair("iswitness", false)); if (pwallet && pwallet->GetCScript(scriptID, subscript)) { - std::vector addresses; - txnouttype whichType; - int nRequired; - ExtractDestinations(subscript, whichType, addresses, nRequired); - obj.push_back(Pair("script", GetTxnOutputType(whichType))); - obj.push_back(Pair("hex", HexStr(subscript.begin(), subscript.end()))); - UniValue a(UniValue::VARR); - for (const CTxDestination& addr : addresses) { - a.push_back(EncodeDestination(addr)); - } - obj.push_back(Pair("addresses", a)); - if (whichType == TX_MULTISIG) - obj.push_back(Pair("sigsrequired", nRequired)); + ProcessSubScript(subscript, obj, true); } return obj; } @@ -103,7 +131,7 @@ class DescribeAddressVisitor : public boost::static_visitor uint160 hash; hasher.Write(id.begin(), 32).Finalize(hash.begin()); if (pwallet && pwallet->GetCScript(CScriptID(hash), subscript)) { - obj.push_back(Pair("hex", HexStr(subscript.begin(), subscript.end()))); + ProcessSubScript(subscript, obj); } return obj; } @@ -131,23 +159,32 @@ UniValue validateaddress(const JSONRPCRequest& request) "\nResult:\n" "{\n" " \"isvalid\" : true|false, (boolean) If the address is valid or not. If not, this is the only property returned.\n" - " \"address\" : \"address\", (string) The bitcoin address validated\n" + " \"address\" : \"address\", (string) The bitcoin 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" " \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n" - " \"isscript\" : true|false, (boolean) If the key is a script\n" - " \"script\" : \"type\" (string, optional) The output script type. Possible types: nonstandard, pubkey, pubkeyhash, scripthash, multisig, nulldata, witness_v0_keyhash, witness_v0_scripthash\n" - " \"hex\" : \"hex\", (string, optional) The redeemscript for the p2sh address\n" - " \"addresses\" (string, optional) Array of addresses associated with the known redeemscript\n" + " \"isscript\" : true|false, (boolean, optional) If the address is P2SH or P2WSH. Not included for unknown witness types.\n" + " \"iswitness\" : true|false, (boolean) If the address is P2WPKH, P2WSH, or an unknown witness version\n" + " \"witness_version\" : version (number, optional) For all witness output types, gives the version number.\n" + " \"witness_program\" : \"hex\" (string, optional) For all witness output types, gives the script or key hash present in the address.\n" + " \"script\" : \"type\" (string, optional) The output script type. Only if \"isscript\" is true and the redeemscript is known. Possible types: nonstandard, pubkey, pubkeyhash, scripthash, multisig, nulldata, witness_v0_keyhash, witness_v0_scripthash, witness_unknown\n" + " \"hex\" : \"hex\", (string, optional) The redeemscript for the P2SH or P2WSH address\n" + " \"addresses\" (string, optional) Array of addresses associated with the known redeemscript (only if \"iswitness\" is false). This field is superseded by the \"pubkeys\" field and the address inside \"embedded\".\n" " [\n" " \"address\"\n" " ,...\n" " ]\n" - " \"sigsrequired\" : xxxxx (numeric, optional) Number of signatures required to spend multisig output\n" - " \"pubkey\" : \"publickeyhex\", (string) The hex value of the raw public key\n" + " \"pubkeys\" (string, optional) Array of pubkeys associated with the known redeemscript (only if \"script\" is \"multisig\")\n" + " [\n" + " \"pubkey\"\n" + " ,...\n" + " ]\n" + " \"sigsrequired\" : xxxxx (numeric, optional) Number of signatures required to spend multisig output (only if \"script\" is \"multisig\")\n" + " \"pubkey\" : \"publickeyhex\", (string, optional) The hex value of the raw public key, for single-key addresses (possibly embedded in P2SH or P2WSH)\n" + " \"embedded\" : {...}, (object, optional) information about the address embedded in P2SH or P2WSH, if relevant and known. It includes all validateaddress output fields for the embedded address, excluding \"isvalid\", metadata (\"timestamp\", \"hdkeypath\", \"hdmasterkeyid\") and relation to the wallet (\"ismine\", \"iswatchonly\", \"account\").\n" " \"iscompressed\" : true|false, (boolean) If the address is compressed\n" " \"account\" : \"account\" (string) DEPRECATED. The account associated with the address, \"\" is the default account\n" - " \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\n" + " \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\n" " \"hdkeypath\" : \"keypath\" (string, optional) The HD keypath if the key is HD and available\n" " \"hdmasterkeyid\" : \"\" (string, optional) The Hash160 of the HD master pubkey\n" "}\n" @@ -188,8 +225,9 @@ UniValue validateaddress(const JSONRPCRequest& request) } if (pwallet) { const CKeyMetadata* meta = nullptr; - if (const CKeyID* key_id = boost::get(&dest)) { - auto it = pwallet->mapKeyMetadata.find(*key_id); + CKeyID key_id = GetKeyForDestination(*pwallet, dest); + if (!key_id.IsNull()) { + auto it = pwallet->mapKeyMetadata.find(key_id); if (it != pwallet->mapKeyMetadata.end()) { meta = &it->second; } diff --git a/src/script/standard.h b/src/script/standard.h index b1c314fa72c5d..f46d6922597f9 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -155,6 +155,10 @@ bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet) * addressRet is populated with a single value and nRequiredRet is set to 1. * Returns true if successful. Currently does not extract address from * pay-to-witness scripts. + * + * Note: this function confuses destinations (a subset of CScripts that are + * encodable as an address) with key identifiers (of keys involved in a + * CScript), and its use should be phased out. */ bool ExtractDestinations(const CScript& scriptPubKey, txnouttype& typeRet, std::vector& addressRet, int& nRequiredRet); From 37c03d3e05713e14458ca2867e442926f165afea Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:48:49 -0800 Subject: [PATCH 06/12] Support P2WPKH addresses in create/addmultisig --- src/rpc/misc.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index f36879f3b188e..826ac9d6bc069 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -280,12 +280,12 @@ CScript _createmultisig_redeemScript(CWallet * const pwallet, const UniValue& pa // Case 1: Bitcoin address and we have full public key: CTxDestination dest = DecodeDestination(ks); if (pwallet && IsValidDestination(dest)) { - const CKeyID *keyID = boost::get(&dest); - if (!keyID) { + CKeyID key = GetKeyForDestination(*pwallet, dest); + if (key.IsNull()) { throw std::runtime_error(strprintf("%s does not refer to a key", ks)); } CPubKey vchPubKey; - if (!pwallet->GetPubKey(*keyID, vchPubKey)) { + if (!pwallet->GetPubKey(key, vchPubKey)) { throw std::runtime_error(strprintf("no full public key for address %s", ks)); } if (!vchPubKey.IsFullyValid()) From cf2c0b6f5cde584d7004ff1b5b476ef54de6b74b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:48:55 -0800 Subject: [PATCH 07/12] Support P2WPKH and P2SH-P2WPKH in dumpprivkey --- src/wallet/rpcdump.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 71d50be63425b..81922c5848edf 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -578,12 +578,12 @@ UniValue dumpprivkey(const JSONRPCRequest& request) if (!IsValidDestination(dest)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); } - const CKeyID *keyID = boost::get(&dest); - if (!keyID) { + auto keyid = GetKeyForDestination(*pwallet, dest); + if (keyid.IsNull()) { throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to a key"); } CKey vchSecret; - if (!pwallet->GetKey(*keyID, vchSecret)) { + if (!pwallet->GetKey(keyid, vchSecret)) { throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } return CBitcoinSecret(vchSecret).ToString(); From 57273f2b302949d4ca3511f703627b5d717be40c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:49:01 -0800 Subject: [PATCH 08/12] [test] Serialize CTransaction with witness by default --- test/functional/p2p-fullblocktest.py | 7 ++++-- test/functional/p2p-segwit.py | 2 +- test/functional/test_framework/messages.py | 25 ++++++++++++---------- test/functional/test_framework/script.py | 2 +- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/test/functional/p2p-fullblocktest.py b/test/functional/p2p-fullblocktest.py index 010dbdccad312..8d85a4a93470a 100755 --- a/test/functional/p2p-fullblocktest.py +++ b/test/functional/p2p-fullblocktest.py @@ -36,12 +36,15 @@ def initialize(self, base_block): self.vtx = copy.deepcopy(base_block.vtx) self.hashMerkleRoot = self.calc_merkle_root() - def serialize(self): + def serialize(self, with_witness=False): r = b"" r += super(CBlock, self).serialize() r += struct.pack(" Date: Thu, 30 Nov 2017 16:49:04 -0800 Subject: [PATCH 09/12] Implicitly know about P2WPKH redeemscripts Make CKeyStore automatically known about the redeemscripts necessary for P2SH-P2WPKH (and due to the extra checks in IsMine, also P2WPKH) spending. --- src/keystore.cpp | 35 +++++++++++++++++++++++++++-- src/keystore.h | 2 ++ src/test/script_standard_tests.cpp | 7 +----- src/wallet/crypter.cpp | 1 + src/wallet/rpcwallet.cpp | 2 +- test/functional/segwit.py | 36 +++++++++++++++++------------- 6 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index cbcc6d7ca9a88..487784b0bfc57 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -11,6 +11,31 @@ bool CKeyStore::AddKey(const CKey &key) { return AddKeyPubKey(key, key.GetPubKey()); } +void CBasicKeyStore::ImplicitlyLearnRelatedKeyScripts(const CPubKey& pubkey) +{ + AssertLockHeld(cs_KeyStore); + CKeyID key_id = pubkey.GetID(); + // We must actually know about this key already. + assert(HaveKey(key_id) || mapWatchKeys.count(key_id)); + // This adds the redeemscripts necessary to detect P2WPKH and P2SH-P2WPKH + // outputs. Technically P2WPKH outputs don't have a redeemscript to be + // spent. However, our current IsMine logic requires the corresponding + // P2SH-P2WPKH redeemscript to be present in the wallet in order to accept + // payment even to P2WPKH outputs. + // Also note that having superfluous scripts in the keystore never hurts. + // They're only used to guide recursion in signing and IsMine logic - if + // a script is present but we can't do anything with it, it has no effect. + // "Implicitly" refers to fact that scripts are derived automatically from + // existing keys, and are present in memory, even without being explicitly + // loaded (e.g. from a file). + if (pubkey.IsCompressed()) { + CScript script = GetScriptForDestination(WitnessV0KeyHash(key_id)); + // This does not use AddCScript, as it may be overridden. + CScriptID id(script); + mapScripts[id] = std::move(script); + } +} + bool CBasicKeyStore::GetPubKey(const CKeyID &address, CPubKey &vchPubKeyOut) const { CKey key; @@ -31,6 +56,7 @@ bool CBasicKeyStore::AddKeyPubKey(const CKey& key, const CPubKey &pubkey) { LOCK(cs_KeyStore); mapKeys[pubkey.GetID()] = key; + ImplicitlyLearnRelatedKeyScripts(pubkey); return true; } @@ -110,8 +136,10 @@ bool CBasicKeyStore::AddWatchOnly(const CScript &dest) LOCK(cs_KeyStore); setWatchOnly.insert(dest); CPubKey pubKey; - if (ExtractPubKey(dest, pubKey)) + if (ExtractPubKey(dest, pubKey)) { mapWatchKeys[pubKey.GetID()] = pubKey; + ImplicitlyLearnRelatedKeyScripts(pubKey); + } return true; } @@ -120,8 +148,11 @@ bool CBasicKeyStore::RemoveWatchOnly(const CScript &dest) LOCK(cs_KeyStore); setWatchOnly.erase(dest); CPubKey pubKey; - if (ExtractPubKey(dest, pubKey)) + if (ExtractPubKey(dest, pubKey)) { mapWatchKeys.erase(pubKey.GetID()); + } + // Related CScripts are not removed; having superfluous scripts around is + // harmless (see comment in ImplicitlyLearnRelatedKeyScripts). return true; } diff --git a/src/keystore.h b/src/keystore.h index 0f3b5706f21b3..b8656424be254 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -59,6 +59,8 @@ class CBasicKeyStore : public CKeyStore ScriptMap mapScripts; WatchOnlySet setWatchOnly; + void ImplicitlyLearnRelatedKeyScripts(const CPubKey& pubkey); + public: bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey) override; bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const override; diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 19060eccc9395..cd30fbeda7264 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -508,12 +508,7 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine) scriptPubKey.clear(); scriptPubKey << OP_0 << ToByteVector(pubkeys[0].GetID()); - // Keystore has key, but no P2SH redeemScript - result = IsMine(keystore, scriptPubKey, isInvalid); - BOOST_CHECK_EQUAL(result, ISMINE_NO); - BOOST_CHECK(!isInvalid); - - // Keystore has key and P2SH redeemScript + // Keystore implicitly has key and P2SH redeemScript keystore.AddCScript(scriptPubKey); result = IsMine(keystore, scriptPubKey, isInvalid); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 4cd7db048b4f4..72ed5af42dbf8 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -245,6 +245,7 @@ bool CCryptoKeyStore::AddCryptedKey(const CPubKey &vchPubKey, const std::vector< } mapCryptedKeys[vchPubKey.GetID()] = make_pair(vchPubKey, vchCryptedSecret); + ImplicitlyLearnRelatedKeyScripts(vchPubKey); return true; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b366992dec2d0..e4b2b020b75b3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1307,7 +1307,7 @@ UniValue addwitnessaddress(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Cannot convert between witness address types"); } } else { - pwallet->AddCScript(witprogram); + pwallet->AddCScript(witprogram); // Implicit for single-key now, but necessary for multisig and for compatibility with older software pwallet->SetAddressBook(w.result, "", "receive"); } diff --git a/test/functional/segwit.py b/test/functional/segwit.py index 338fa1bc526ac..626572336bafd 100755 --- a/test/functional/segwit.py +++ b/test/functional/segwit.py @@ -356,8 +356,10 @@ def run_test(self): [p2wpkh, p2sh_p2wpkh, p2pk, p2pkh, p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh] = self.p2pkh_address_to_script(v) # normal P2PKH and P2PK with compressed keys should always be spendable spendable_anytime.extend([p2pkh, p2pk]) - # P2SH_P2PK, P2SH_P2PKH, and witness with compressed keys are spendable after direct importaddress - spendable_after_importaddress.extend([p2wpkh, p2sh_p2wpkh, p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh]) + # P2SH_P2PK, P2SH_P2PKH with compressed keys are spendable after direct importaddress + spendable_after_importaddress.extend([p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh]) + # P2WPKH and P2SH_P2WPKH with compressed keys should always be spendable + spendable_anytime.extend([p2wpkh, p2sh_p2wpkh]) for i in uncompressed_spendable_address: v = self.nodes[0].validateaddress(i) @@ -373,7 +375,7 @@ def run_test(self): spendable_anytime.extend([p2pkh, p2pk]) # P2SH_P2PK and P2SH_P2PKH are spendable after direct importaddress spendable_after_importaddress.extend([p2sh_p2pk, p2sh_p2pkh]) - # witness with uncompressed keys are never seen + # Witness output types with uncompressed keys are never seen unseen_anytime.extend([p2wpkh, p2sh_p2wpkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh]) for i in compressed_solvable_address: @@ -384,10 +386,10 @@ def run_test(self): solvable_after_importaddress.extend([bare, p2sh, p2wsh, p2sh_p2wsh]) else: [p2wpkh, p2sh_p2wpkh, p2pk, p2pkh, p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh] = self.p2pkh_address_to_script(v) - # normal P2PKH and P2PK with compressed keys should always be seen - solvable_anytime.extend([p2pkh, p2pk]) - # P2SH_P2PK, P2SH_P2PKH, and witness with compressed keys are seen after direct importaddress - solvable_after_importaddress.extend([p2wpkh, p2sh_p2wpkh, p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh]) + # normal P2PKH, P2PK, P2WPKH and P2SH_P2WPKH with compressed keys should always be seen + solvable_anytime.extend([p2pkh, p2pk, p2wpkh, p2sh_p2wpkh]) + # P2SH_P2PK, P2SH_P2PKH with compressed keys are seen after direct importaddress + solvable_after_importaddress.extend([p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh]) for i in uncompressed_solvable_address: v = self.nodes[0].validateaddress(i) @@ -403,7 +405,7 @@ def run_test(self): solvable_anytime.extend([p2pkh, p2pk]) # P2SH_P2PK, P2SH_P2PKH with uncompressed keys are seen after direct importaddress solvable_after_importaddress.extend([p2sh_p2pk, p2sh_p2pkh]) - # witness with uncompressed keys are never seen + # Witness output types with uncompressed keys are never seen unseen_anytime.extend([p2wpkh, p2sh_p2wpkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh]) op1 = CScript([OP_1]) @@ -496,6 +498,8 @@ def run_test(self): spendable_after_addwitnessaddress = [] # These outputs should be seen after importaddress solvable_after_addwitnessaddress=[] # These outputs should be seen after importaddress but not spendable unseen_anytime = [] # These outputs should never be seen + solvable_anytime = [] # These outputs should be solvable after importpubkey + unseen_anytime = [] # These outputs should never be seen uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]])) uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]])) @@ -514,9 +518,8 @@ def run_test(self): premature_witaddress.append(script_to_p2sh(p2wsh)) else: [p2wpkh, p2sh_p2wpkh, p2pk, p2pkh, p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh] = self.p2pkh_address_to_script(v) - # P2WPKH, P2SH_P2WPKH are spendable after addwitnessaddress - spendable_after_addwitnessaddress.extend([p2wpkh, p2sh_p2wpkh]) - premature_witaddress.append(script_to_p2sh(p2wpkh)) + # P2WPKH, P2SH_P2WPKH are always spendable + spendable_anytime.extend([p2wpkh, p2sh_p2wpkh]) for i in uncompressed_spendable_address + uncompressed_solvable_address: v = self.nodes[0].validateaddress(i) @@ -538,10 +541,11 @@ def run_test(self): premature_witaddress.append(script_to_p2sh(p2wsh)) else: [p2wpkh, p2sh_p2wpkh, p2pk, p2pkh, p2sh_p2pk, p2sh_p2pkh, p2wsh_p2pk, p2wsh_p2pkh, p2sh_p2wsh_p2pk, p2sh_p2wsh_p2pkh] = self.p2pkh_address_to_script(v) - # P2SH_P2PK, P2SH_P2PKH with compressed keys are seen after addwitnessaddress - solvable_after_addwitnessaddress.extend([p2wpkh, p2sh_p2wpkh]) - premature_witaddress.append(script_to_p2sh(p2wpkh)) + # P2SH_P2PK, P2SH_P2PKH with compressed keys are always solvable + solvable_anytime.extend([p2wpkh, p2sh_p2wpkh]) + self.mine_and_test_listunspent(spendable_anytime, 2) + self.mine_and_test_listunspent(solvable_anytime, 1) self.mine_and_test_listunspent(spendable_after_addwitnessaddress + solvable_after_addwitnessaddress + unseen_anytime, 0) # addwitnessaddress should refuse to return a witness address if an uncompressed key is used @@ -558,8 +562,8 @@ def run_test(self): witaddress = self.nodes[0].addwitnessaddress(i) assert_equal(witaddress, self.nodes[0].addwitnessaddress(witaddress)) - spendable_txid.append(self.mine_and_test_listunspent(spendable_after_addwitnessaddress, 2)) - solvable_txid.append(self.mine_and_test_listunspent(solvable_after_addwitnessaddress, 1)) + spendable_txid.append(self.mine_and_test_listunspent(spendable_after_addwitnessaddress + spendable_anytime, 2)) + solvable_txid.append(self.mine_and_test_listunspent(solvable_after_addwitnessaddress + solvable_anytime, 1)) self.mine_and_test_listunspent(unseen_anytime, 0) # Check that createrawtransaction/decoderawtransaction with non-v0 Bech32 works From 940a21932ba769ba5829cba713579db84f96d2f8 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:49:11 -0800 Subject: [PATCH 10/12] SegWit wallet support This introduces two command line flags (-addresstype and -changetype) which control the type of addresses/outputs created by the GUI and RPCs. Certain RPCs allow overriding these (`getnewaddress` and `getrawchangeaddress`). Supported types are "legacy" (P2PKH and P2SH-multisig), "p2sh-segwit" (P2SH-P2WPKH and P2SH-P2WSH-multisig), and "bech32" (P2WPKH and P2WSH-multisig). A few utility functions are added to the wallet to construct different address type and to add the necessary entries to the wallet file to be compatible with earlier versions (see `CWallet::LearnRelatedScripts`, `GetDestinationForKey`, `GetAllDestinationsForKey`, `CWallet::AddAndGetDestinationForScript`). --- src/qt/addresstablemodel.cpp | 3 +- src/qt/paymentserver.cpp | 35 ++++--- src/qt/test/wallettests.cpp | 5 +- src/wallet/init.cpp | 12 +++ src/wallet/rpcdump.cpp | 12 ++- src/wallet/rpcwallet.cpp | 50 +++++++--- src/wallet/test/wallet_test_fixture.cpp | 2 + src/wallet/wallet.cpp | 117 +++++++++++++++++++++++- src/wallet/wallet.h | 45 +++++++++ test/functional/bip68-112-113-p2p.py | 2 +- test/functional/bip68-sequence.py | 3 +- test/functional/bumpfee.py | 3 +- test/functional/import-rescan.py | 2 +- test/functional/importmulti.py | 1 + test/functional/nulldummy.py | 2 +- test/functional/rawtransactions.py | 5 +- test/functional/segwit.py | 10 +- test/functional/signmessages.py | 1 + test/functional/test_runner.py | 1 + test/functional/txn_clone.py | 15 ++- test/functional/wallet-dump.py | 2 +- test/functional/wallet.py | 11 ++- 22 files changed, 278 insertions(+), 61 deletions(-) diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 1d16940acb9f5..f238c37f635cf 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -384,7 +384,8 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con return QString(); } } - strAddress = EncodeDestination(newKey.GetID()); + wallet->LearnRelatedScripts(newKey, g_address_type); + strAddress = EncodeDestination(GetDestinationForKey(newKey, g_address_type)); } else { diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 510a3783ae5a2..48462f9b5facc 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -636,27 +636,24 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, const SendCoinsRecipient& r // Create a new refund address, or re-use: QString account = tr("Refund from %1").arg(recipient.authenticatedMerchant); std::string strAccount = account.toStdString(); - std::set refundAddresses = wallet->GetAccountAddresses(strAccount); - if (!refundAddresses.empty()) { - CScript s = GetScriptForDestination(*refundAddresses.begin()); + CPubKey newKey; + if (wallet->GetKeyFromPool(newKey)) { + // BIP70 requests encode the scriptPubKey directly, so we are not restricted to address + // types supported by the receiver. As a result, we choose the address format we also + // use for change. Despite an actual payment and not change, this is a close match: + // it's the output type we use subject to privacy issues, but not restricted by what + // other software supports. + wallet->LearnRelatedScripts(newKey, g_change_type); + CTxDestination dest = GetDestinationForKey(newKey, g_change_type); + wallet->SetAddressBook(dest, strAccount, "refund"); + + CScript s = GetScriptForDestination(dest); payments::Output* refund_to = payment.add_refund_to(); refund_to->set_script(&s[0], s.size()); - } - else { - CPubKey newKey; - if (wallet->GetKeyFromPool(newKey)) { - CKeyID keyID = newKey.GetID(); - wallet->SetAddressBook(keyID, strAccount, "refund"); - - CScript s = GetScriptForDestination(keyID); - payments::Output* refund_to = payment.add_refund_to(); - refund_to->set_script(&s[0], s.size()); - } - else { - // This should never happen, because sending coins should have - // just unlocked the wallet and refilled the keypool. - qWarning() << "PaymentServer::fetchPaymentACK: Error getting refund key, refund_to not set"; - } + } else { + // This should never happen, because sending coins should have + // just unlocked the wallet and refilled the keypool. + qWarning() << "PaymentServer::fetchPaymentACK: Error getting refund key, refund_to not set"; } int length = payment.ByteSize(); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 4b7c3bd726a6d..a270e5de594fc 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -149,6 +149,9 @@ void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, st // src/qt/test/test_bitcoin-qt -platform cocoa # macOS void TestGUI() { + g_address_type = OUTPUT_TYPE_P2SH_SEGWIT; + g_change_type = OUTPUT_TYPE_P2SH_SEGWIT; + // Set up wallet and chain with 105 blocks (5 mature blocks for spending). TestChain100Setup test; for (int i = 0; i < 5; ++i) { @@ -161,7 +164,7 @@ void TestGUI() wallet.LoadWallet(firstRun); { LOCK(wallet.cs_wallet); - wallet.SetAddressBook(test.coinbaseKey.GetPubKey().GetID(), "", "receive"); + wallet.SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), g_address_type), "", "receive"); wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); } { diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 67c46df87d69c..8da99f2f7ea48 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -16,6 +16,8 @@ std::string GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); + strUsage += HelpMessageOpt("-addresstype", strprintf(_("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")"), FormatOutputType(OUTPUT_TYPE_DEFAULT))); + strUsage += HelpMessageOpt("-changetype", _("What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default is same as -addresstype)")); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); strUsage += HelpMessageOpt("-keypool=", strprintf(_("Set key pool size to (default: %u)"), DEFAULT_KEYPOOL_SIZE)); strUsage += HelpMessageOpt("-fallbackfee=", strprintf(_("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)"), @@ -175,6 +177,16 @@ bool WalletParameterInteraction() bSpendZeroConfChange = gArgs.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); fWalletRbf = gArgs.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); + g_address_type = ParseOutputType(gArgs.GetArg("-addresstype", "")); + if (g_address_type == OUTPUT_TYPE_NONE) { + return InitError(strprintf(_("Unknown address type '%s'"), gArgs.GetArg("-addresstype", ""))); + } + + g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), g_address_type); + if (g_change_type == OUTPUT_TYPE_NONE) { + return InitError(strprintf(_("Unknown change type '%s'"), gArgs.GetArg("-changetype", ""))); + } + return true; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 81922c5848edf..17d81a5e39ea4 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -131,7 +131,11 @@ UniValue importprivkey(const JSONRPCRequest& request) CKeyID vchAddress = pubkey.GetID(); { pwallet->MarkDirty(); - pwallet->SetAddressBook(vchAddress, strLabel, "receive"); + + // We don't know which corresponding address will be used; label them all + for (const auto& dest : GetAllDestinationsForKey(pubkey)) { + pwallet->SetAddressBook(dest, strLabel, "receive"); + } // Don't throw error in case a key is already there if (pwallet->HaveKey(vchAddress)) { @@ -143,6 +147,7 @@ UniValue importprivkey(const JSONRPCRequest& request) if (!pwallet->AddKeyPubKey(key, pubkey)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } + pwallet->LearnAllRelatedScripts(pubkey); // whenever a key is imported, we need to scan the whole chain pwallet->UpdateTimeFirstKey(1); @@ -433,8 +438,11 @@ UniValue importpubkey(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - ImportAddress(pwallet, pubKey.GetID(), strLabel); + for (const auto& dest : GetAllDestinationsForKey(pubKey)) { + ImportAddress(pwallet, dest, strLabel); + } ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false); + pwallet->LearnAllRelatedScripts(pubKey); if (fRescan) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e4b2b020b75b3..447dd794ae9bf 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -136,14 +136,15 @@ UniValue getnewaddress(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() > 1) + if (request.fHelp || request.params.size() > 2) throw std::runtime_error( - "getnewaddress ( \"account\" )\n" + "getnewaddress ( \"account\" \"address_type\" )\n" "\nReturns a new Bitcoin address for receiving payments.\n" "If 'account' is specified (DEPRECATED), it is added to the address book \n" "so payments received with the address will be credited to 'account'.\n" "\nArguments:\n" "1. \"account\" (string, optional) DEPRECATED. The account name for the address to be linked to. If not provided, the default account \"\" is used. It can also be set to the empty string \"\" to represent the default account. The account does not need to exist, it will be created if there is no account by the given name.\n" + "2. \"address_type\" (string, optional) The address type to use. Options are \"legacy\", \"p2sh\", and \"bech32\". Default is set by -addresstype.\n" "\nResult:\n" "\"address\" (string) The new bitcoin address\n" "\nExamples:\n" @@ -158,6 +159,14 @@ UniValue getnewaddress(const JSONRPCRequest& request) if (!request.params[0].isNull()) strAccount = AccountFromValue(request.params[0]); + OutputType output_type = g_address_type; + if (!request.params[1].isNull()) { + output_type = ParseOutputType(request.params[1].get_str(), g_address_type); + if (output_type == OUTPUT_TYPE_NONE) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str())); + } + } + if (!pwallet->IsLocked()) { pwallet->TopUpKeyPool(); } @@ -167,11 +176,12 @@ UniValue getnewaddress(const JSONRPCRequest& request) if (!pwallet->GetKeyFromPool(newKey)) { throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); } - CKeyID keyID = newKey.GetID(); + pwallet->LearnRelatedScripts(newKey, output_type); + CTxDestination dest = GetDestinationForKey(newKey, output_type); - pwallet->SetAddressBook(keyID, strAccount, "receive"); + pwallet->SetAddressBook(dest, strAccount, "receive"); - return EncodeDestination(keyID); + return EncodeDestination(dest); } @@ -226,11 +236,13 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() > 0) + if (request.fHelp || request.params.size() > 1) throw std::runtime_error( - "getrawchangeaddress\n" + "getrawchangeaddress ( \"address_type\" )\n" "\nReturns a new Bitcoin address, for receiving change.\n" "This is for use with raw transactions, NOT normal use.\n" + "\nArguments:\n" + "1. \"address_type\" (string, optional) The address type to use. Options are \"legacy\", \"p2sh\", and \"bech32\". Default is set by -changetype.\n" "\nResult:\n" "\"address\" (string) The address\n" "\nExamples:\n" @@ -244,6 +256,14 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); } + OutputType output_type = g_change_type; + if (!request.params[0].isNull()) { + output_type = ParseOutputType(request.params[0].get_str(), g_change_type); + if (output_type == OUTPUT_TYPE_NONE) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); + } + } + CReserveKey reservekey(pwallet); CPubKey vchPubKey; if (!reservekey.GetReservedKey(vchPubKey, true)) @@ -251,9 +271,10 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) reservekey.KeepKey(); - CKeyID keyID = vchPubKey.GetID(); + pwallet->LearnRelatedScripts(vchPubKey, output_type); + CTxDestination dest = GetDestinationForKey(vchPubKey, output_type); - return EncodeDestination(keyID); + return EncodeDestination(dest); } @@ -1184,11 +1205,12 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) // Construct using pay-to-script-hash: CScript inner = _createmultisig_redeemScript(pwallet, request.params); - CScriptID innerID(inner); pwallet->AddCScript(inner); - pwallet->SetAddressBook(innerID, strAccount, "send"); - return EncodeDestination(innerID); + CTxDestination dest = pwallet->AddAndGetDestinationForScript(inner, g_address_type); + + pwallet->SetAddressBook(dest, strAccount, "send"); + return EncodeDestination(dest); } class Witnessifier : public boost::static_visitor @@ -3446,8 +3468,8 @@ static const CRPCCommand commands[] = { "wallet", "getaccount", &getaccount, {"address"} }, { "wallet", "getaddressesbyaccount", &getaddressesbyaccount, {"account"} }, { "wallet", "getbalance", &getbalance, {"account","minconf","include_watchonly"} }, - { "wallet", "getnewaddress", &getnewaddress, {"account"} }, - { "wallet", "getrawchangeaddress", &getrawchangeaddress, {} }, + { "wallet", "getnewaddress", &getnewaddress, {"account","address_type"} }, + { "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} }, { "wallet", "getreceivedbyaccount", &getreceivedbyaccount, {"account","minconf"} }, { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} }, { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly"} }, diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 3ee83d2d7c28d..86f4df6008f78 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -13,6 +13,8 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): bitdb.MakeMock(); bool fFirstRun; + g_address_type = OUTPUT_TYPE_DEFAULT; + g_change_type = OUTPUT_TYPE_DEFAULT; std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); pwalletMain = MakeUnique(std::move(dbw)); pwalletMain->LoadWallet(fFirstRun); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f73f787259a31..ff052c75bea86 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -42,6 +42,8 @@ CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE); unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET; bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE; bool fWalletRbf = DEFAULT_WALLET_RBF; +OutputType g_address_type = OUTPUT_TYPE_NONE; +OutputType g_change_type = OUTPUT_TYPE_NONE; const char * DEFAULT_WALLET_DAT = "wallet.dat"; const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; @@ -832,8 +834,8 @@ bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount if (!account.vchPubKey.IsValid()) bForceNew = true; else { - // Check if the current key has been used - CScript scriptPubKey = GetScriptForDestination(account.vchPubKey.GetID()); + // Check if the current key has been used (TODO: check other addresses with the same key) + CScript scriptPubKey = GetScriptForDestination(GetDestinationForKey(account.vchPubKey, g_address_type)); for (std::map::iterator it = mapWallet.begin(); it != mapWallet.end() && account.vchPubKey.IsValid(); ++it) @@ -850,11 +852,12 @@ bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount if (!GetKeyFromPool(account.vchPubKey, false)) return false; - dest = account.vchPubKey.GetID(); + LearnRelatedScripts(account.vchPubKey, g_address_type); + dest = GetDestinationForKey(account.vchPubKey, g_address_type); SetAddressBook(dest, strAccount, "receive"); walletdb.WriteAccount(strAccount, account); } else { - dest = account.vchPubKey.GetID(); + dest = GetDestinationForKey(account.vchPubKey, g_address_type); } return true; @@ -2743,7 +2746,8 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT return false; } - scriptChange = GetScriptForDestination(vchPubKey.GetID()); + LearnRelatedScripts(vchPubKey, g_change_type); + scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type)); } CTxOut change_prototype_txout(0, scriptChange); size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); @@ -4136,3 +4140,106 @@ bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& fInMempool = ret; return ret; } + +static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy"; +static const std::string OUTPUT_TYPE_STRING_P2SH_SEGWIT = "p2sh-segwit"; +static const std::string OUTPUT_TYPE_STRING_BECH32 = "bech32"; + +OutputType ParseOutputType(const std::string& type, OutputType default_type) +{ + if (type.empty()) { + return default_type; + } else if (type == OUTPUT_TYPE_STRING_LEGACY) { + return OUTPUT_TYPE_LEGACY; + } else if (type == OUTPUT_TYPE_STRING_P2SH_SEGWIT) { + return OUTPUT_TYPE_P2SH_SEGWIT; + } else if (type == OUTPUT_TYPE_STRING_BECH32) { + return OUTPUT_TYPE_BECH32; + } else { + return OUTPUT_TYPE_NONE; + } +} + +const std::string& FormatOutputType(OutputType type) +{ + switch (type) { + case OUTPUT_TYPE_LEGACY: return OUTPUT_TYPE_STRING_LEGACY; + case OUTPUT_TYPE_P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; + case OUTPUT_TYPE_BECH32: return OUTPUT_TYPE_STRING_BECH32; + default: assert(false); + } +} + +void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type) +{ + if (key.IsCompressed() && (type == OUTPUT_TYPE_P2SH_SEGWIT || type == OUTPUT_TYPE_BECH32)) { + CTxDestination witdest = WitnessV0KeyHash(key.GetID()); + CScript witprog = GetScriptForDestination(witdest); + // Make sure the resulting program is solvable. + assert(IsSolvable(*this, witprog)); + AddCScript(witprog); + } +} + +void CWallet::LearnAllRelatedScripts(const CPubKey& key) +{ + // OUTPUT_TYPE_P2SH_SEGWIT always adds all necessary scripts for all types. + LearnRelatedScripts(key, OUTPUT_TYPE_P2SH_SEGWIT); +} + +CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) +{ + switch (type) { + case OUTPUT_TYPE_LEGACY: return key.GetID(); + case OUTPUT_TYPE_P2SH_SEGWIT: + case OUTPUT_TYPE_BECH32: { + if (!key.IsCompressed()) return key.GetID(); + CTxDestination witdest = WitnessV0KeyHash(key.GetID()); + CScript witprog = GetScriptForDestination(witdest); + if (type == OUTPUT_TYPE_P2SH_SEGWIT) { + return CScriptID(witprog); + } else { + return witdest; + } + } + default: assert(false); + } +} + +std::vector GetAllDestinationsForKey(const CPubKey& key) +{ + CKeyID keyid = key.GetID(); + if (key.IsCompressed()) { + CTxDestination segwit = WitnessV0KeyHash(keyid); + CTxDestination p2sh = CScriptID(GetScriptForDestination(segwit)); + return std::vector{std::move(keyid), std::move(p2sh), std::move(segwit)}; + } else { + return std::vector{std::move(keyid)}; + } +} + +CTxDestination CWallet::AddAndGetDestinationForScript(const CScript& script, OutputType type) +{ + // Note that scripts over 520 bytes are not yet supported. + switch (type) { + case OUTPUT_TYPE_LEGACY: + return CScriptID(script); + case OUTPUT_TYPE_P2SH_SEGWIT: + case OUTPUT_TYPE_BECH32: { + WitnessV0ScriptHash hash; + CSHA256().Write(script.data(), script.size()).Finalize(hash.begin()); + CTxDestination witdest = hash; + CScript witprog = GetScriptForDestination(witdest); + // Check if the resulting program is solvable (i.e. doesn't use an uncompressed key) + if (!IsSolvable(*this, witprog)) return CScriptID(script); + // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours. + AddCScript(witprog); + if (type == OUTPUT_TYPE_BECH32) { + return witdest; + } else { + return CScriptID(witprog); + } + } + default: assert(false); + } +} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e64ea2534afec..4088ed198565b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -99,6 +99,19 @@ enum WalletFeature FEATURE_LATEST = FEATURE_COMPRPUBKEY // HD is optional, use FEATURE_COMPRPUBKEY as latest version }; +enum OutputType +{ + OUTPUT_TYPE_NONE, + OUTPUT_TYPE_LEGACY, + OUTPUT_TYPE_P2SH_SEGWIT, + OUTPUT_TYPE_BECH32, + + OUTPUT_TYPE_DEFAULT = OUTPUT_TYPE_P2SH_SEGWIT +}; + +extern OutputType g_address_type; +extern OutputType g_change_type; + /** A key pool entry */ class CKeyPool @@ -1129,6 +1142,26 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface * deadlock */ void BlockUntilSyncedToCurrentChain(); + + /** + * Explicitly make the wallet learn the related scripts for outputs to the + * given key. This is purely to make the wallet file compatible with older + * software, as CBasicKeyStore automatically does this implicitly for all + * keys now. + */ + void LearnRelatedScripts(const CPubKey& key, OutputType); + + /** + * Same as LearnRelatedScripts, but when the OutputType is not known (and could + * be anything). + */ + void LearnAllRelatedScripts(const CPubKey& key); + + /** + * Get a destination of the requested type (if possible) to the specified script. + * This function will automatically add the necessary scripts to the wallet. + */ + CTxDestination AddAndGetDestinationForScript(const CScript& script, OutputType); }; /** A key allocated from the key pool. */ @@ -1218,4 +1251,16 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const ContainerType &coins return true; } +OutputType ParseOutputType(const std::string& str, OutputType default_type = OUTPUT_TYPE_DEFAULT); +const std::string& FormatOutputType(OutputType type); + +/** + * Get a destination of the requested type (if possible) to the specified key. + * The caller must make sure LearnRelatedScripts has been called beforehand. + */ +CTxDestination GetDestinationForKey(const CPubKey& key, OutputType); + +/** Get all destinations (potentially) supported by the wallet for the given key. */ +std::vector GetAllDestinationsForKey(const CPubKey& key); + #endif // BITCOIN_WALLET_WALLET_H diff --git a/test/functional/bip68-112-113-p2p.py b/test/functional/bip68-112-113-p2p.py index d3c7d8fc1138a..4cecfabc7a6a9 100755 --- a/test/functional/bip68-112-113-p2p.py +++ b/test/functional/bip68-112-113-p2p.py @@ -95,7 +95,7 @@ class BIP68_112_113Test(ComparisonTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True - self.extra_args = [['-whitelist=127.0.0.1', '-blockversion=4']] + self.extra_args = [['-whitelist=127.0.0.1', '-blockversion=4', '-addresstype=legacy']] def run_test(self): test = TestManager(self, self.options.tmpdir) diff --git a/test/functional/bip68-sequence.py b/test/functional/bip68-sequence.py index 5f8f21701f934..14603054206d0 100755 --- a/test/functional/bip68-sequence.py +++ b/test/functional/bip68-sequence.py @@ -362,9 +362,10 @@ def test_bip68_not_consensus(self): block.vtx.extend([tx1, tx2, tx3]) block.hashMerkleRoot = block.calc_merkle_root() block.rehash() + add_witness_commitment(block) block.solve() - self.nodes[0].submitblock(ToHex(block)) + self.nodes[0].submitblock(bytes_to_hex_str(block.serialize(True))) assert_equal(self.nodes[0].getbestblockhash(), block.hash) def activateCSV(self): diff --git a/test/functional/bumpfee.py b/test/functional/bumpfee.py index 008e83d5b2a7e..eb76801c591e9 100755 --- a/test/functional/bumpfee.py +++ b/test/functional/bumpfee.py @@ -194,7 +194,7 @@ def test_settxfee(rbf_node, dest_address): requested_feerate = Decimal("0.00025000") rbf_node.settxfee(requested_feerate) bumped_tx = rbf_node.bumpfee(rbfid) - actual_feerate = bumped_tx["fee"] * 1000 / rbf_node.getrawtransaction(bumped_tx["txid"], True)["size"] + actual_feerate = bumped_tx["fee"] * 1000 / rbf_node.getrawtransaction(bumped_tx["txid"], True)["vsize"] # Assert that the difference between the requested feerate and the actual # feerate of the bumped transaction is small. assert_greater_than(Decimal("0.00001000"), abs(requested_feerate - actual_feerate)) @@ -290,6 +290,7 @@ def submit_block_with_tx(node, tx): block.vtx.append(ctx) block.rehash() block.hashMerkleRoot = block.calc_merkle_root() + blocktools.add_witness_commitment(block) block.solve() node.submitblock(bytes_to_hex_str(block.serialize(True))) return block diff --git a/test/functional/import-rescan.py b/test/functional/import-rescan.py index 6807fa66964db..4306a8e22715f 100755 --- a/test/functional/import-rescan.py +++ b/test/functional/import-rescan.py @@ -119,7 +119,7 @@ def set_test_params(self): self.num_nodes = 2 + len(IMPORT_NODES) def setup_network(self): - extra_args = [[] for _ in range(self.num_nodes)] + extra_args = [["-addresstype=legacy"] for _ in range(self.num_nodes)] for i, import_node in enumerate(IMPORT_NODES, 2): if import_node.prune: extra_args[i] += ["-prune=1"] diff --git a/test/functional/importmulti.py b/test/functional/importmulti.py index a691595f15116..922b829440fe0 100755 --- a/test/functional/importmulti.py +++ b/test/functional/importmulti.py @@ -9,6 +9,7 @@ class ImportMultiTest (BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 + self.extra_args = [["-addresstype=legacy"], ["-addresstype=legacy"]] self.setup_clean_chain = True def setup_network(self): diff --git a/test/functional/nulldummy.py b/test/functional/nulldummy.py index 9f9f2f90c0ff4..2426ebedbc46e 100755 --- a/test/functional/nulldummy.py +++ b/test/functional/nulldummy.py @@ -42,7 +42,7 @@ def set_test_params(self): self.setup_clean_chain = True # This script tests NULLDUMMY activation, which is part of the 'segwit' deployment, so we go through # normal segwit activation here (and don't use the default always-on behaviour). - self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness', '-vbparams=segwit:0:999999999999']] + self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness', '-vbparams=segwit:0:999999999999', '-addresstype=legacy']] def run_test(self): self.address = self.nodes[0].getnewaddress() diff --git a/test/functional/rawtransactions.py b/test/functional/rawtransactions.py index 79f2a2834ea96..1ff8d8a944aef 100755 --- a/test/functional/rawtransactions.py +++ b/test/functional/rawtransactions.py @@ -20,6 +20,7 @@ class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 + self.extra_args = [["-addresstype=legacy"], ["-addresstype=legacy"], ["-addresstype=legacy"]] def setup_network(self, split=False): super().setup_network() @@ -135,7 +136,7 @@ def run_test(self): break bal = self.nodes[0].getbalance() - inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex']}] + inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "amount" : vout['value']}] outputs = { self.nodes[0].getnewaddress() : 2.19 } rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxPartialSigned = self.nodes[1].signrawtransaction(rawTx, inputs) @@ -180,7 +181,7 @@ def run_test(self): break bal = self.nodes[0].getbalance() - inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex']}] + inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex'], "amount" : vout['value']}] outputs = { self.nodes[0].getnewaddress() : 2.19 } rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs) rawTxPartialSigned1 = self.nodes[1].signrawtransaction(rawTx2, inputs) diff --git a/test/functional/segwit.py b/test/functional/segwit.py index 626572336bafd..f4b2e4dc53327 100755 --- a/test/functional/segwit.py +++ b/test/functional/segwit.py @@ -78,9 +78,9 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. - self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0", "-vbparams=segwit:0:999999999999"], - ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999"], - ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-vbparams=segwit:0:999999999999"]] + self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy"], + ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy"], + ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-vbparams=segwit:0:999999999999", "-addresstype=legacy"]] def setup_network(self): super().setup_network() @@ -135,9 +135,9 @@ def run_test(self): self.pubkey.append(self.nodes[i].validateaddress(newaddress)["pubkey"]) multiaddress = self.nodes[i].addmultisigaddress(1, [self.pubkey[-1]]) multiscript = CScript([OP_1, hex_str_to_bytes(self.pubkey[-1]), OP_1, OP_CHECKMULTISIG]) - p2sh_addr = self.nodes[i].addwitnessaddress(newaddress, True) + p2sh_addr = self.nodes[i].addwitnessaddress(newaddress) bip173_addr = self.nodes[i].addwitnessaddress(newaddress, False) - p2sh_ms_addr = self.nodes[i].addwitnessaddress(multiaddress, True) + p2sh_ms_addr = self.nodes[i].addwitnessaddress(multiaddress) bip173_ms_addr = self.nodes[i].addwitnessaddress(multiaddress, False) assert_equal(p2sh_addr, key_to_p2sh_p2wpkh(self.pubkey[-1])) assert_equal(bip173_addr, key_to_p2wpkh(self.pubkey[-1])) diff --git a/test/functional/signmessages.py b/test/functional/signmessages.py index 52ba6a5ad7618..10449a7d6f3b3 100755 --- a/test/functional/signmessages.py +++ b/test/functional/signmessages.py @@ -11,6 +11,7 @@ class SignMessagesTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 + self.extra_args = [["-addresstype=legacy"]] def run_test(self): message = 'This is just a test message' diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 58faec521d74f..6b83e862bbf92 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -86,6 +86,7 @@ 'mempool_resurrect_test.py', 'txn_doublespend.py --mineblock', 'txn_clone.py', + 'txn_clone.py --segwit', 'getchaintips.py', 'rest.py', 'mempool_spendcoinbase.py', diff --git a/test/functional/txn_clone.py b/test/functional/txn_clone.py index 740bb2d4c5c06..20351625d3d8b 100755 --- a/test/functional/txn_clone.py +++ b/test/functional/txn_clone.py @@ -14,6 +14,8 @@ def set_test_params(self): def add_options(self, parser): parser.add_option("--mineblock", dest="mine_block", default=False, action="store_true", help="Test double-spend of 1-confirmed transaction") + parser.add_option("--segwit", dest="segwit", default=False, action="store_true", + help="Test behaviour with SegWit txn (which should fail") def setup_network(self): # Start with split network: @@ -22,6 +24,11 @@ def setup_network(self): disconnect_nodes(self.nodes[2], 1) def run_test(self): + if self.options.segwit: + output_type="p2sh-segwit" + else: + output_type="legacy" + # All nodes should start with 1,250 BTC: starting_balance = 1250 for i in range(4): @@ -31,11 +38,11 @@ def run_test(self): # Assign coins to foo and bar accounts: self.nodes[0].settxfee(.001) - node0_address_foo = self.nodes[0].getnewaddress("foo") + node0_address_foo = self.nodes[0].getnewaddress("foo", output_type) fund_foo_txid = self.nodes[0].sendfrom("", node0_address_foo, 1219) fund_foo_tx = self.nodes[0].gettransaction(fund_foo_txid) - node0_address_bar = self.nodes[0].getnewaddress("bar") + node0_address_bar = self.nodes[0].getnewaddress("bar", output_type) fund_bar_txid = self.nodes[0].sendfrom("", node0_address_bar, 29) fund_bar_tx = self.nodes[0].gettransaction(fund_bar_txid) @@ -106,6 +113,10 @@ def run_test(self): # Send clone and its parent to miner self.nodes[2].sendrawtransaction(fund_foo_tx["hex"]) txid1_clone = self.nodes[2].sendrawtransaction(tx1_clone["hex"]) + if self.options.segwit: + assert_equal(txid1, txid1_clone) + return + # ... mine a block... self.nodes[2].generate(1) diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py index 47de8777a6558..f1b7828e5c2ce 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -58,7 +58,7 @@ def read_dump(file_name, addrs, hd_master_addr_old): class WalletDumpTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [["-keypool=90"]] + self.extra_args = [["-keypool=90", "-addresstype=legacy"]] def setup_network(self, split=False): # Use 1 minute timeout because the initial getnewaddress RPC can take diff --git a/test/functional/wallet.py b/test/functional/wallet.py index db60df18ede00..68264c4c7aa18 100755 --- a/test/functional/wallet.py +++ b/test/functional/wallet.py @@ -27,6 +27,9 @@ def check_fee_amount(self, curr_balance, balance_with_fee, fee_per_byte, tx_size assert_fee_amount(fee, tx_size, fee_per_byte * 1000) return curr_balance + def get_vsize(self, txn): + return self.nodes[0].decoderawtransaction(txn)['vsize'] + def run_test(self): # Check that there's no UTXO on none of the nodes assert_equal(len(self.nodes[0].listunspent()), 0) @@ -162,7 +165,7 @@ def run_test(self): txid = self.nodes[2].sendtoaddress(address, 10, "", "", False) self.nodes[2].generate(1) self.sync_all([self.nodes[0:3]]) - node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), Decimal('84'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid))) + node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), Decimal('84'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) assert_equal(self.nodes[0].getbalance(), Decimal('10')) # Send 10 BTC with subtract fee from amount @@ -171,14 +174,14 @@ def run_test(self): self.sync_all([self.nodes[0:3]]) node_2_bal -= Decimal('10') assert_equal(self.nodes[2].getbalance(), node_2_bal) - node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('20'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid))) + node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('20'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) # Sendmany 10 BTC txid = self.nodes[2].sendmany('from1', {address: 10}, 0, "", []) self.nodes[2].generate(1) self.sync_all([self.nodes[0:3]]) node_0_bal += Decimal('10') - node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid))) + node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) assert_equal(self.nodes[0].getbalance(), node_0_bal) # Sendmany 10 BTC with subtract fee from amount @@ -187,7 +190,7 @@ def run_test(self): self.sync_all([self.nodes[0:3]]) node_2_bal -= Decimal('10') assert_equal(self.nodes[2].getbalance(), node_2_bal) - node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, count_bytes(self.nodes[2].getrawtransaction(txid))) + node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) # Test ResendWalletTransactions: # Create a couple of transactions, then start up a fourth From 7ee54fd7c7fa31e3cec382651dd373f7ef2d7327 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 30 Nov 2017 16:49:16 -0800 Subject: [PATCH 11/12] Support downgrading after recovered keypool witness keys --- src/wallet/wallet.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ff052c75bea86..e473f402c4602 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3636,6 +3636,7 @@ void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id) if (walletdb.ReadPool(index, keypool)) { //TODO: This should be unnecessary m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); } + LearnAllRelatedScripts(keypool.vchPubKey); walletdb.ErasePool(index); LogPrintf("keypool index %d removed\n", index); it = setKeyPool->erase(it); From b224a47a1a5feac380506abff63fae91d7a93b39 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 2 Dec 2017 13:03:53 -0800 Subject: [PATCH 12/12] Add address_types test Improvements and cleanups by John Newbery --- test/functional/address_types.py | 199 +++++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 200 insertions(+) create mode 100755 test/functional/address_types.py diff --git a/test/functional/address_types.py b/test/functional/address_types.py new file mode 100755 index 0000000000000..cb119c04b0d79 --- /dev/null +++ b/test/functional/address_types.py @@ -0,0 +1,199 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 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 that the wallet can send and receive using all combinations of address types. + +There are 4 nodes-under-test: + - node0 uses legacy addresses + - node1 uses p2sh/segwit addresses + - node2 uses p2sh/segwit addresses and bech32 addresses for change + - node3 uses bech32 addresses + +node4 exists to generate new blocks. + +The script is a series of tests, iterating over the 4 nodes. In each iteration +of the test, one node sends: + - 10/101th of its balance to itself (using getrawchangeaddress for single key addresses) + - 20/101th to the next node + - 30/101th to the node after that + - 40/101th to the remaining node + - 1/101th remains as fee+change + +Iterate over each node for single key addresses, and then over each node for +multisig addresses. In a second iteration, the same is done, but with explicit address_type +parameters passed to getnewaddress and getrawchangeaddress. Node0 and node3 send to p2sh, +node 1 sends to bech32, and node2 sends to legacy. As every node sends coins after receiving, +this also verifies that spending coins sent to all these address types works.""" + +from decimal import Decimal +import itertools + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_greater_than, connect_nodes_bi, sync_blocks, sync_mempools + +class AddressTypeTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 5 + self.extra_args = [["-addresstype=legacy"], ["-addresstype=p2sh-segwit"], ["-addresstype=p2sh-segwit", "-changetype=bech32"], ["-addresstype=bech32"], []] + + def setup_network(self): + self.setup_nodes() + + # Fully mesh-connect nodes for faster mempool sync + for i, j in itertools.product(range(self.num_nodes), repeat=2): + if i > j: + connect_nodes_bi(self.nodes, i, j) + self.sync_all() + + def get_balances(self, confirmed=True): + """Return a list of confirmed or unconfirmed balances.""" + if confirmed: + return [self.nodes[i].getbalance() for i in range(4)] + else: + return [self.nodes[i].getunconfirmedbalance() for i in range(4)] + + def test_address(self, node, address, multisig, typ): + """Run sanity checks on an address.""" + info = self.nodes[node].validateaddress(address) + assert(info['isvalid']) + if not multisig and typ == 'legacy': + # P2PKH + assert(not info['isscript']) + assert(not info['iswitness']) + assert('pubkey' in info) + elif not multisig and typ == 'p2sh-segwit': + # P2SH-P2WPKH + assert(info['isscript']) + assert(not info['iswitness']) + assert_equal(info['script'], 'witness_v0_keyhash') + assert('pubkey' in info) + elif not multisig and typ == 'bech32': + # P2WPKH + assert(not info['isscript']) + assert(info['iswitness']) + assert_equal(info['witness_version'], 0) + assert_equal(len(info['witness_program']), 40) + assert('pubkey' in info) + elif typ == 'legacy': + # P2SH-multisig + assert(info['isscript']) + assert_equal(info['script'], 'multisig') + assert(not info['iswitness']) + assert('pubkeys' in info) + elif typ == 'p2sh-segwit': + # P2SH-P2WSH-multisig + assert(info['isscript']) + assert_equal(info['script'], 'witness_v0_scripthash') + assert(not info['iswitness']) + assert(info['embedded']['isscript']) + assert_equal(info['embedded']['script'], 'multisig') + assert(info['embedded']['iswitness']) + assert_equal(info['embedded']['witness_version'], 0) + assert_equal(len(info['embedded']['witness_program']), 64) + assert('pubkeys' in info['embedded']) + elif typ == 'bech32': + # P2WSH-multisig + assert(info['isscript']) + assert_equal(info['script'], 'multisig') + assert(info['iswitness']) + assert_equal(info['witness_version'], 0) + assert_equal(len(info['witness_program']), 64) + assert('pubkeys' in info) + else: + # Unknown type + assert(False) + + def run_test(self): + # Mine 101 blocks on node4 to bring nodes out of IBD and make sure that + # no coinbases are maturing for the nodes-under-test during the test + self.nodes[4].generate(101) + sync_blocks(self.nodes) + + uncompressed_1 = "0496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858ee" + uncompressed_2 = "047211a824f55b505228e4c3d5194c1fcfaa15a456abdf37f9b9d97a4040afc073dee6c89064984f03385237d92167c13e236446b417ab79a0fcae412ae3316b77" + compressed_1 = "0296b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52" + compressed_2 = "037211a824f55b505228e4c3d5194c1fcfaa15a456abdf37f9b9d97a4040afc073" + + # addmultisigaddress with at least 1 uncompressed key should return a legacy address. + for node in range(4): + self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, uncompressed_2]), True, 'legacy') + self.test_address(node, self.nodes[node].addmultisigaddress(2, [compressed_1, uncompressed_2]), True, 'legacy') + self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, compressed_2]), True, 'legacy') + # addmultisigaddress with all compressed keys should return the appropriate address type (even when the keys are not ours). + self.test_address(0, self.nodes[0].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'legacy') + self.test_address(1, self.nodes[1].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'p2sh-segwit') + self.test_address(2, self.nodes[2].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'p2sh-segwit') + self.test_address(3, self.nodes[3].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'bech32') + + for explicit_type, multisig, from_node in itertools.product([False, True], [False, True], range(4)): + address_type = None + if explicit_type and not multisig: + if from_node == 1: + address_type = 'bech32' + elif from_node == 0 or from_node == 3: + address_type = 'p2sh-segwit' + else: + address_type = 'legacy' + self.log.info("Sending from node {} ({}) with{} multisig using {}".format(from_node, self.extra_args[from_node], "" if multisig else "out", "default" if address_type is None else address_type)) + old_balances = self.get_balances() + self.log.debug("Old balances are {}".format(old_balances)) + to_send = (old_balances[from_node] / 101).quantize(Decimal("0.00000001")) + sends = {} + + self.log.debug("Prepare sends") + for n, to_node in enumerate(range(from_node, from_node + 4)): + to_node %= 4 + change = False + if not multisig: + if from_node == to_node: + # When sending non-multisig to self, use getrawchangeaddress + address = self.nodes[to_node].getrawchangeaddress(address_type=address_type) + change = True + else: + address = self.nodes[to_node].getnewaddress(address_type=address_type) + else: + addr1 = self.nodes[to_node].getnewaddress() + addr2 = self.nodes[to_node].getnewaddress() + address = self.nodes[to_node].addmultisigaddress(2, [addr1, addr2]) + + # Do some sanity checking on the created address + if address_type is not None: + typ = address_type + elif to_node == 0: + typ = 'legacy' + elif to_node == 1 or (to_node == 2 and not change): + typ = 'p2sh-segwit' + else: + typ = 'bech32' + self.test_address(to_node, address, multisig, typ) + + # Output entry + sends[address] = to_send * 10 * (1 + n) + + self.log.debug("Sending: {}".format(sends)) + self.nodes[from_node].sendmany("", sends) + sync_mempools(self.nodes) + + unconf_balances = self.get_balances(False) + self.log.debug("Check unconfirmed balances: {}".format(unconf_balances)) + assert_equal(unconf_balances[from_node], 0) + for n, to_node in enumerate(range(from_node + 1, from_node + 4)): + to_node %= 4 + assert_equal(unconf_balances[to_node], to_send * 10 * (2 + n)) + + # node4 collects fee and block subsidy to keep accounting simple + self.nodes[4].generate(1) + sync_blocks(self.nodes) + + new_balances = self.get_balances() + self.log.debug("Check new balances: {}".format(new_balances)) + # We don't know what fee was set, so we can only check bounds on the balance of the sending node + assert_greater_than(new_balances[from_node], to_send * 10) + assert_greater_than(to_send * 11, new_balances[from_node]) + for n, to_node in enumerate(range(from_node + 1, from_node + 4)): + to_node %= 4 + assert_equal(new_balances[to_node], old_balances[to_node] + to_send * 10 * (2 + n)) + +if __name__ == '__main__': + AddressTypeTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 6b83e862bbf92..bed12fa324723 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -78,6 +78,7 @@ 'abandonconflict.py', 'bip68-112-113-p2p.py', 'rawtransactions.py', + 'address_types.py', 'reindex.py', # vv Tests less than 30s vv 'keypool-topup.py',