diff options
30 files changed, 756 insertions, 412 deletions
diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md index 139c03181f..6dc277f298 100644 --- a/contrib/seeds/README.md +++ b/contrib/seeds/README.md @@ -4,7 +4,9 @@ Utility to generate the seeds.txt list that is compiled into the client (see [src/chainparamsseeds.h](/src/chainparamsseeds.h) and other utilities in [contrib/seeds](/contrib/seeds)). Be sure to update `PATTERN_AGENT` in `makeseeds.py` to include the current version, -and remove old versions as necessary. +and remove old versions as necessary (at a minimum when GetDesireableServiceFlags +changes its default return value, as those are the services which seeds are added +to addrman with). The seeds compiled into the release are created from sipa's DNS seed data, like this: diff --git a/src/Makefile.am b/src/Makefile.am index 4b65774fc6..4fbd605d9e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -133,6 +133,7 @@ BITCOIN_CORE_H = \ rpc/safemode.h \ rpc/server.h \ rpc/register.h \ + rpc/util.h \ scheduler.h \ script/sigcache.h \ script/sign.h \ @@ -352,6 +353,7 @@ libbitcoin_util_a_SOURCES = \ fs.cpp \ random.cpp \ rpc/protocol.cpp \ + rpc/util.cpp \ support/cleanse.cpp \ sync.cpp \ threadinterrupt.cpp \ diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 97a6c346ab..39757baf9e 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -124,13 +124,17 @@ public: assert(consensus.hashGenesisBlock == uint256S("0x000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f")); assert(genesis.hashMerkleRoot == uint256S("0x4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b")); - // Note that of those with the service bits flag, most only support a subset of possible options - vSeeds.emplace_back("seed.bitcoin.sipa.be", true); // Pieter Wuille, only supports x1, x5, x9, and xd - vSeeds.emplace_back("dnsseed.bluematt.me", true); // Matt Corallo, only supports x9 - vSeeds.emplace_back("dnsseed.bitcoin.dashjr.org", false); // Luke Dashjr - vSeeds.emplace_back("seed.bitcoinstats.com", true); // Christian Decker, supports x1 - xf - vSeeds.emplace_back("seed.bitcoin.jonasschnelli.ch", true); // Jonas Schnelli, only supports x1, x5, x9, and xd - vSeeds.emplace_back("seed.btc.petertodd.org", true); // Peter Todd, only supports x1, x5, x9, and xd + // Note that of those which support the service bits prefix, most only support a subset of + // possible options. + // This is fine at runtime as we'll fall back to using them as a oneshot if they dont support the + // service bits we want, but we should get them updated to support all service bits wanted by any + // release ASAP to avoid it where possible. + vSeeds.emplace_back("seed.bitcoin.sipa.be"); // Pieter Wuille, only supports x1, x5, x9, and xd + vSeeds.emplace_back("dnsseed.bluematt.me"); // Matt Corallo, only supports x9 + vSeeds.emplace_back("dnsseed.bitcoin.dashjr.org"); // Luke Dashjr + vSeeds.emplace_back("seed.bitcoinstats.com"); // Christian Decker, supports x1 - xf + vSeeds.emplace_back("seed.bitcoin.jonasschnelli.ch"); // Jonas Schnelli, only supports x1, x5, x9, and xd + vSeeds.emplace_back("seed.btc.petertodd.org"); // Peter Todd, only supports x1, x5, x9, and xd base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,0); base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,5); @@ -229,10 +233,10 @@ public: vFixedSeeds.clear(); vSeeds.clear(); // nodes with support for servicebits filtering should be at the top - vSeeds.emplace_back("testnet-seed.bitcoin.jonasschnelli.ch", true); - vSeeds.emplace_back("seed.tbtc.petertodd.org", true); - vSeeds.emplace_back("seed.testnet.bitcoin.sprovoost.nl", true); - vSeeds.emplace_back("testnet-seed.bluematt.me", false); + vSeeds.emplace_back("testnet-seed.bitcoin.jonasschnelli.ch"); + vSeeds.emplace_back("seed.tbtc.petertodd.org"); + vSeeds.emplace_back("seed.testnet.bitcoin.sprovoost.nl"); + vSeeds.emplace_back("testnet-seed.bluematt.me"); // Just a static list of stable node(s), only supports x9 base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,111); base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,196); diff --git a/src/chainparams.h b/src/chainparams.h index ecfe1a0ac3..d478da9891 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -14,12 +14,6 @@ #include <memory> #include <vector> -struct CDNSSeedData { - std::string host; - bool supportsServiceBitsFiltering; - CDNSSeedData(const std::string &strHost, bool supportsServiceBitsFilteringIn) : host(strHost), supportsServiceBitsFiltering(supportsServiceBitsFilteringIn) {} -}; - struct SeedSpec6 { uint8_t addr[16]; uint16_t port; @@ -71,7 +65,8 @@ public: bool MineBlocksOnDemand() const { return fMineBlocksOnDemand; } /** Return the BIP70 network string (main, test or regtest) */ std::string NetworkIDString() const { return strNetworkID; } - const std::vector<CDNSSeedData>& DNSSeeds() const { return vSeeds; } + /** Return the list of hostnames to look up for DNS seeds */ + const std::vector<std::string>& DNSSeeds() const { return vSeeds; } const std::vector<unsigned char>& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } const std::string& Bech32HRP() const { return bech32_hrp; } const std::vector<SeedSpec6>& FixedSeeds() const { return vFixedSeeds; } @@ -85,7 +80,7 @@ protected: CMessageHeader::MessageStartChars pchMessageStart; int nDefaultPort; uint64_t nPruneAfterHeight; - std::vector<CDNSSeedData> vSeeds; + std::vector<std::string> vSeeds; std::vector<unsigned char> base58Prefixes[MAX_BASE58_TYPES]; std::string bech32_hrp; std::string strNetworkID; diff --git a/src/net.cpp b/src/net.cpp index 682f47bff2..8111390749 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -136,7 +136,7 @@ static std::vector<CAddress> convertSeed6(const std::vector<SeedSpec6> &vSeedsIn for (const auto& seed_in : vSeedsIn) { struct in6_addr ip; memcpy(&ip, seed_in.addr, sizeof(ip)); - CAddress addr(CService(ip, seed_in.port), NODE_NETWORK); + CAddress addr(CService(ip, seed_in.port), GetDesirableServiceFlags(NODE_NONE)); addr.nTime = GetTime() - GetRand(nOneWeek) - nOneWeek; vSeedsOut.push_back(addr); } @@ -1577,19 +1577,6 @@ void MapPort(bool) -static std::string GetDNSHost(const CDNSSeedData& data, ServiceFlags* requiredServiceBits) -{ - //use default host for non-filter-capable seeds or if we use the default service bits (NODE_NETWORK) - if (!data.supportsServiceBitsFiltering || *requiredServiceBits == NODE_NETWORK) { - *requiredServiceBits = NODE_NETWORK; - return data.host; - } - - // See chainparams.cpp, most dnsseeds only support one or two possible servicebits hostnames - return strprintf("x%x.%s", *requiredServiceBits, data.host); -} - - void CConnman::ThreadDNSAddressSeed() { // goal: only query DNS seeds if address need is acute @@ -1612,22 +1599,22 @@ void CConnman::ThreadDNSAddressSeed() } } - const std::vector<CDNSSeedData> &vSeeds = Params().DNSSeeds(); + const std::vector<std::string> &vSeeds = Params().DNSSeeds(); int found = 0; LogPrintf("Loading addresses from DNS seeds (could take a while)\n"); - for (const CDNSSeedData &seed : vSeeds) { + for (const std::string &seed : vSeeds) { if (interruptNet) { return; } if (HaveNameProxy()) { - AddOneShot(seed.host); + AddOneShot(seed); } else { std::vector<CNetAddr> vIPs; std::vector<CAddress> vAdd; ServiceFlags requiredServiceBits = GetDesirableServiceFlags(NODE_NONE); - std::string host = GetDNSHost(seed, &requiredServiceBits); + std::string host = strprintf("x%x.%s", requiredServiceBits, seed); CNetAddr resolveSource; if (!resolveSource.SetInternal(host)) { continue; @@ -1643,6 +1630,10 @@ void CConnman::ThreadDNSAddressSeed() found++; } addrman.Add(vAdd, resolveSource); + } else { + // We now avoid directly using results from DNS Seeds which do not support service bit filtering, + // instead using them as a oneshot to get nodes with our desired service bits. + AddOneShot(seed); } } } diff --git a/src/protocol.h b/src/protocol.h index 452b5b28ce..43d8ac067a 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -291,7 +291,15 @@ enum ServiceFlags : uint64_t { * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which * case NODE_NETWORK_LIMITED suffices). * - * Thus, generally, avoid calling with peerServices == NODE_NONE. + * Thus, generally, avoid calling with peerServices == NODE_NONE, unless + * state-specific flags must absolutely be avoided. When called with + * peerServices == NODE_NONE, the returned desirable service flags are + * guaranteed to not change dependant on state - ie they are suitable for + * use when describing peers which we know to be desirable, but for which + * we do not have a confirmed set of service flags. + * + * If the NODE_NONE return value is changed, contrib/seeds/makeseeds.py + * should be updated appropriately to filter for the same nodes. */ static ServiceFlags GetDesirableServiceFlags(ServiceFlags services) { return ServiceFlags(NODE_NETWORK | NODE_WITNESS); diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index dc729649b8..bc69d4f945 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -643,8 +643,9 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, const SendCoinsRecipient& r // 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); + const OutputType change_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type; + wallet->LearnRelatedScripts(newKey, change_type); + CTxDestination dest = GetDestinationForKey(newKey, change_type); wallet->SetAddressBook(dest, strAccount, "refund"); CScript s = GetScriptForDestination(dest); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index a270e5de59..cd49292138 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -169,7 +169,9 @@ void TestGUI() } { LOCK(cs_main); - wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, true); + WalletRescanReserver reserver(&wallet); + reserver.reserve(); + wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true); } wallet.SetBroadcastTransactions(true); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 3bcad16316..c11dda22c4 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -15,6 +15,7 @@ #include <netbase.h> #include <rpc/blockchain.h> #include <rpc/server.h> +#include <rpc/util.h> #include <timedata.h> #include <util.h> #include <utilstrencodings.h> @@ -254,88 +255,21 @@ UniValue validateaddress(const JSONRPCRequest& request) // Needed even with !ENABLE_WALLET, to pass (ignored) pointers around class CWallet; -/** - * Used by addmultisigaddress / createmultisig: - */ -CScript _createmultisig_redeemScript(CWallet * const pwallet, const UniValue& params) -{ - int nRequired = params[0].get_int(); - const UniValue& keys = params[1].get_array(); - - // Gather public keys - if (nRequired < 1) - throw std::runtime_error("a multisignature address must require at least one key to redeem"); - if ((int)keys.size() < nRequired) - throw std::runtime_error( - strprintf("not enough keys supplied " - "(got %u keys, but need at least %d to redeem)", keys.size(), nRequired)); - if (keys.size() > 16) - throw std::runtime_error("Number of addresses involved in the multisignature address creation > 16\nReduce the number"); - std::vector<CPubKey> pubkeys; - pubkeys.resize(keys.size()); - for (unsigned int i = 0; i < keys.size(); i++) - { - const std::string& ks = keys[i].get_str(); -#ifdef ENABLE_WALLET - // Case 1: Bitcoin address and we have full public key: - CTxDestination dest = DecodeDestination(ks); - if (pwallet && IsValidDestination(dest)) { - 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(key, vchPubKey)) { - throw std::runtime_error(strprintf("no full public key for address %s", ks)); - } - if (!vchPubKey.IsFullyValid()) - throw std::runtime_error(" Invalid public key: "+ks); - pubkeys[i] = vchPubKey; - } - - // Case 2: hex public key - else -#endif - if (IsHex(ks)) - { - CPubKey vchPubKey(ParseHex(ks)); - if (!vchPubKey.IsFullyValid()) - throw std::runtime_error(" Invalid public key: "+ks); - pubkeys[i] = vchPubKey; - } - else - { - throw std::runtime_error(" Invalid public key: "+ks); - } - } - CScript result = GetScriptForMultisig(nRequired, pubkeys); - - if (result.size() > MAX_SCRIPT_ELEMENT_SIZE) - throw std::runtime_error( - strprintf("redeemScript exceeds size limit: %d > %d", result.size(), MAX_SCRIPT_ELEMENT_SIZE)); - - return result; -} - UniValue createmultisig(const JSONRPCRequest& request) { -#ifdef ENABLE_WALLET - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); -#else - CWallet * const pwallet = nullptr; -#endif - if (request.fHelp || request.params.size() < 2 || request.params.size() > 2) { std::string msg = "createmultisig nrequired [\"key\",...]\n" "\nCreates a multi-signature address with n signature of m keys required.\n" "It returns a json object with the address and redeemScript.\n" - + "DEPRECATION WARNING: Using addresses with createmultisig is deprecated. Clients must\n" + "transition to using addmultisigaddress to create multisig addresses with addresses known\n" + "to the wallet before upgrading to v0.17. To use the deprecated functionality, start bitcoind with -deprecatedrpc=createmultisig\n" "\nArguments:\n" - "1. nrequired (numeric, required) The number of required signatures out of the n keys or addresses.\n" - "2. \"keys\" (string, required) A json array of keys which are bitcoin addresses or hex-encoded public keys\n" + "1. nrequired (numeric, required) The number of required signatures out of the n keys or addresses.\n" + "2. \"keys\" (string, required) A json array of hex-encoded public keys\n" " [\n" - " \"key\" (string) bitcoin address or hex-encoded public key\n" + " \"key\" (string) The hex-encoded public key\n" " ,...\n" " ]\n" @@ -346,16 +280,37 @@ UniValue createmultisig(const JSONRPCRequest& request) "}\n" "\nExamples:\n" - "\nCreate a multisig address from 2 addresses\n" - + HelpExampleCli("createmultisig", "2 \"[\\\"16sSauSf5pF2UkUwvKGq4qjNRzBZYqgEL5\\\",\\\"171sgjn4YtPu27adkKGrdDwzRTxnRkBfKV\\\"]\"") + + "\nCreate a multisig address from 2 public keys\n" + + HelpExampleCli("createmultisig", "2 \"[\\\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\\\",\\\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\\\"]\"") + "\nAs a json rpc call\n" - + HelpExampleRpc("createmultisig", "2, \"[\\\"16sSauSf5pF2UkUwvKGq4qjNRzBZYqgEL5\\\",\\\"171sgjn4YtPu27adkKGrdDwzRTxnRkBfKV\\\"]\"") + + HelpExampleRpc("createmultisig", "2, \"[\\\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\\\",\\\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\\\"]\"") ; throw std::runtime_error(msg); } + int required = request.params[0].get_int(); + + // Get the public keys + const UniValue& keys = request.params[1].get_array(); + std::vector<CPubKey> pubkeys; + for (unsigned int i = 0; i < keys.size(); ++i) { + if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) { + pubkeys.push_back(HexToPubKey(keys[i].get_str())); + } else { +#ifdef ENABLE_WALLET + CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + if (IsDeprecatedRPCEnabled("createmultisig") && EnsureWalletIsAvailable(pwallet, false)) { + pubkeys.push_back(AddrToPubKey(pwallet, keys[i].get_str())); + } else +#endif + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\nNote that from v0.16, createmultisig no longer accepts addresses." + " Clients must transition to using addmultisigaddress to create multisig addresses with addresses known to the wallet before upgrading to v0.17." + " To use the deprecated functionality, start bitcoind with -deprecatedrpc=createmultisig", keys[i].get_str())); + } + } + // Construct using pay-to-script-hash: - CScript inner = _createmultisig_redeemScript(pwallet, request.params); + CScript inner = CreateMultisigRedeemscript(required, pubkeys); CScriptID innerID(inner); UniValue result(UniValue::VOBJ); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp new file mode 100644 index 0000000000..09ded4e46e --- /dev/null +++ b/src/rpc/util.cpp @@ -0,0 +1,68 @@ +// 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. + +#include <base58.h> +#include <keystore.h> +#include <pubkey.h> +#include <rpc/protocol.h> +#include <rpc/util.h> +#include <tinyformat.h> +#include <utilstrencodings.h> + +// Converts a hex string to a public key if possible +CPubKey HexToPubKey(const std::string& hex_in) +{ + if (!IsHex(hex_in)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + } + CPubKey vchPubKey(ParseHex(hex_in)); + if (!vchPubKey.IsFullyValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + } + return vchPubKey; +} + +// Retrieves a public key for an address from the given CKeyStore +CPubKey AddrToPubKey(CKeyStore* const keystore, const std::string& addr_in) +{ + CTxDestination dest = DecodeDestination(addr_in); + if (!IsValidDestination(dest)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address: " + addr_in); + } + CKeyID key = GetKeyForDestination(*keystore, dest); + if (key.IsNull()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); + } + CPubKey vchPubKey; + if (!keystore->GetPubKey(key, vchPubKey)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("no full public key for address %s", addr_in)); + } + if (!vchPubKey.IsFullyValid()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet contains an invalid public key"); + } + return vchPubKey; +} + +// Creates a multisig redeemscript from a given list of public keys and number required. +CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey>& pubkeys) +{ + // Gather public keys + if (required < 1) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "a multisignature address must require at least one key to redeem"); + } + if ((int)pubkeys.size() < required) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("not enough keys supplied (got %u keys, but need at least %d to redeem)", pubkeys.size(), required)); + } + if (pubkeys.size() > 16) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Number of keys involved in the multisignature address creation > 16\nReduce the number"); + } + + CScript result = GetScriptForMultisig(required, pubkeys); + + if (result.size() > MAX_SCRIPT_ELEMENT_SIZE) { + throw JSONRPCError(RPC_INVALID_PARAMETER, (strprintf("redeemScript exceeds size limit: %d > %d", result.size(), MAX_SCRIPT_ELEMENT_SIZE))); + } + + return result; +} diff --git a/src/rpc/util.h b/src/rpc/util.h new file mode 100644 index 0000000000..568a4260ba --- /dev/null +++ b/src/rpc/util.h @@ -0,0 +1,19 @@ +// 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. + +#ifndef BITCOIN_RPC_UTIL_H +#define BITCOIN_RPC_UTIL_H + +#include <string> +#include <vector> + +class CKeyStore; +class CPubKey; +class CScript; + +CPubKey HexToPubKey(const std::string& hex_in); +CPubKey AddrToPubKey(CKeyStore* const keystore, const std::string& addr_in); +CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey>& pubkeys); + +#endif // BITCOIN_RPC_UTIL_H diff --git a/src/validation.cpp b/src/validation.cpp index 14d60bb269..8cee0dfac3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1121,7 +1121,13 @@ bool ReadBlockFromDisk(CBlock& block, const CDiskBlockPos& pos, const Consensus: bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams) { - if (!ReadBlockFromDisk(block, pindex->GetBlockPos(), consensusParams)) + CDiskBlockPos blockPos; + { + LOCK(cs_main); + blockPos = pindex->GetBlockPos(); + } + + if (!ReadBlockFromDisk(block, blockPos, consensusParams)) return false; if (block.GetHash() != pindex->GetBlockHash()) return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s", diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 2d26f7ae0f..c7f19bc90a 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -17,7 +17,7 @@ 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("-changetype", _("What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\"). Default is same as -addresstype, except when -addresstype=p2sh-segwit a native segwit output is used when sending to a native segwit address)")); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); strUsage += HelpMessageOpt("-keypool=<n>", strprintf(_("Set key pool size to <n> (default: %u)"), DEFAULT_KEYPOOL_SIZE)); strUsage += HelpMessageOpt("-fallbackfee=<amt>", strprintf(_("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)"), @@ -182,8 +182,10 @@ bool WalletParameterInteraction() 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) { + // If changetype is set in config file or parameter, check that it's valid. + // Default to OUTPUT_TYPE_NONE if not set. + g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), OUTPUT_TYPE_NONE); + if (g_change_type == OUTPUT_TYPE_NONE && !gArgs.GetArg("-changetype", "").empty()) { return InitError(strprintf(_("Unknown change type '%s'"), gArgs.GetArg("-changetype", ""))); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 645e776b68..0b021f9fe0 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -86,7 +86,8 @@ UniValue importprivkey(const JSONRPCRequest& request) "1. \"privkey\" (string, required) The private key (see dumpprivkey)\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" - "\nNote: This call can take minutes to complete if rescan is true.\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "\nExamples:\n" "\nDump a private key\n" + HelpExampleCli("dumpprivkey", "\"myaddress\"") + @@ -101,61 +102,65 @@ UniValue importprivkey(const JSONRPCRequest& request) ); - LOCK2(cs_main, pwallet->cs_wallet); - - EnsureWalletIsUnlocked(pwallet); - - std::string strSecret = request.params[0].get_str(); - std::string strLabel = ""; - if (!request.params[1].isNull()) - strLabel = request.params[1].get_str(); - - // Whether to perform rescan after import + WalletRescanReserver reserver(pwallet); bool fRescan = true; - if (!request.params[2].isNull()) - fRescan = request.params[2].get_bool(); - - if (fRescan && fPruneMode) - throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + { + LOCK2(cs_main, pwallet->cs_wallet); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(strSecret); + EnsureWalletIsUnlocked(pwallet); - if (!fGood) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); + std::string strSecret = request.params[0].get_str(); + std::string strLabel = ""; + if (!request.params[1].isNull()) + strLabel = request.params[1].get_str(); - CKey key = vchSecret.GetKey(); - if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + // Whether to perform rescan after import + if (!request.params[2].isNull()) + fRescan = request.params[2].get_bool(); - CPubKey pubkey = key.GetPubKey(); - assert(key.VerifyPubKey(pubkey)); - CKeyID vchAddress = pubkey.GetID(); - { - pwallet->MarkDirty(); + if (fRescan && fPruneMode) + throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); - // We don't know which corresponding address will be used; label them all - for (const auto& dest : GetAllDestinationsForKey(pubkey)) { - pwallet->SetAddressBook(dest, strLabel, "receive"); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } - // Don't throw error in case a key is already there - if (pwallet->HaveKey(vchAddress)) { - return NullUniValue; - } + CBitcoinSecret vchSecret; + bool fGood = vchSecret.SetString(strSecret); - pwallet->mapKeyMetadata[vchAddress].nCreateTime = 1; + if (!fGood) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - if (!pwallet->AddKeyPubKey(key, pubkey)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); - } - pwallet->LearnAllRelatedScripts(pubkey); + CKey key = vchSecret.GetKey(); + if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); - // whenever a key is imported, we need to scan the whole chain - pwallet->UpdateTimeFirstKey(1); + CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); + CKeyID vchAddress = pubkey.GetID(); + { + pwallet->MarkDirty(); + // We don't know which corresponding address will be used; label them all + for (const auto& dest : GetAllDestinationsForKey(pubkey)) { + pwallet->SetAddressBook(dest, strLabel, "receive"); + } - if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + // Don't throw error in case a key is already there + if (pwallet->HaveKey(vchAddress)) { + return NullUniValue; + } + + // whenever a key is imported, we need to scan the whole chain + pwallet->UpdateTimeFirstKey(1); + pwallet->mapKeyMetadata[vchAddress].nCreateTime = 1; + + if (!pwallet->AddKeyPubKey(key, pubkey)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); + } + pwallet->LearnAllRelatedScripts(pubkey); } } + if (fRescan) { + pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); + } return NullUniValue; } @@ -237,7 +242,8 @@ UniValue importaddress(const JSONRPCRequest& request) "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" "4. p2sh (boolean, optional, default=false) Add the P2SH version of the script as well\n" - "\nNote: This call can take minutes to complete if rescan is true.\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" "as change, and not show up in many RPCs.\n" @@ -263,29 +269,35 @@ UniValue importaddress(const JSONRPCRequest& request) if (fRescan && fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + WalletRescanReserver reserver(pwallet); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + // Whether to import a p2sh version, too bool fP2SH = false; if (!request.params[3].isNull()) fP2SH = request.params[3].get_bool(); - LOCK2(cs_main, pwallet->cs_wallet); + { + LOCK2(cs_main, pwallet->cs_wallet); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); - if (IsValidDestination(dest)) { - if (fP2SH) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); + CTxDestination dest = DecodeDestination(request.params[0].get_str()); + if (IsValidDestination(dest)) { + if (fP2SH) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Cannot use the p2sh flag with an address - use a script instead"); + } + ImportAddress(pwallet, dest, strLabel); + } else if (IsHex(request.params[0].get_str())) { + std::vector<unsigned char> data(ParseHex(request.params[0].get_str())); + ImportScript(pwallet, CScript(data.begin(), data.end()), strLabel, fP2SH); + } else { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); } - ImportAddress(pwallet, dest, strLabel); - } else if (IsHex(request.params[0].get_str())) { - std::vector<unsigned char> data(ParseHex(request.params[0].get_str())); - ImportScript(pwallet, CScript(data.begin(), data.end()), strLabel, fP2SH); - } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address or script"); } - if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); pwallet->ReacceptWalletTransactions(); } @@ -406,7 +418,8 @@ UniValue importpubkey(const JSONRPCRequest& request) "1. \"pubkey\" (string, required) The hex-encoded public key\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\n" - "\nNote: This call can take minutes to complete if rescan is true.\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "\nExamples:\n" "\nImport a public key with rescan\n" + HelpExampleCli("importpubkey", "\"mypubkey\"") + @@ -429,6 +442,11 @@ UniValue importpubkey(const JSONRPCRequest& request) if (fRescan && fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + WalletRescanReserver reserver(pwallet); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } + if (!IsHex(request.params[0].get_str())) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string"); std::vector<unsigned char> data(ParseHex(request.params[0].get_str())); @@ -436,17 +454,18 @@ UniValue importpubkey(const JSONRPCRequest& request) if (!pubKey.IsFullyValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); - LOCK2(cs_main, pwallet->cs_wallet); + { + LOCK2(cs_main, pwallet->cs_wallet); - for (const auto& dest : GetAllDestinationsForKey(pubKey)) { - ImportAddress(pwallet, dest, strLabel); + for (const auto& dest : GetAllDestinationsForKey(pubKey)) { + ImportAddress(pwallet, dest, strLabel); + } + ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false); + pwallet->LearnAllRelatedScripts(pubKey); } - ImportScript(pwallet, GetScriptForRawPubKey(pubKey), strLabel, false); - pwallet->LearnAllRelatedScripts(pubKey); - if (fRescan) { - pwallet->RescanFromTime(TIMESTAMP_MIN, true /* update */); + pwallet->RescanFromTime(TIMESTAMP_MIN, reserver, true /* update */); pwallet->ReacceptWalletTransactions(); } @@ -479,91 +498,98 @@ UniValue importwallet(const JSONRPCRequest& request) if (fPruneMode) throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode"); - LOCK2(cs_main, pwallet->cs_wallet); + WalletRescanReserver reserver(pwallet); + if (!reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } - EnsureWalletIsUnlocked(pwallet); + int64_t nTimeBegin = 0; + bool fGood = true; + { + LOCK2(cs_main, pwallet->cs_wallet); - std::ifstream file; - file.open(request.params[0].get_str().c_str(), std::ios::in | std::ios::ate); - if (!file.is_open()) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); + EnsureWalletIsUnlocked(pwallet); - int64_t nTimeBegin = chainActive.Tip()->GetBlockTime(); + std::ifstream file; + file.open(request.params[0].get_str().c_str(), std::ios::in | std::ios::ate); + if (!file.is_open()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); + } + nTimeBegin = chainActive.Tip()->GetBlockTime(); - bool fGood = true; + int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); + file.seekg(0, file.beg); - int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); - file.seekg(0, file.beg); - - pwallet->ShowProgress(_("Importing..."), 0); // show progress dialog in GUI - while (file.good()) { - pwallet->ShowProgress("", std::max(1, std::min(99, (int)(((double)file.tellg() / (double)nFilesize) * 100)))); - std::string line; - std::getline(file, line); - if (line.empty() || line[0] == '#') - continue; - - std::vector<std::string> vstr; - boost::split(vstr, line, boost::is_any_of(" ")); - if (vstr.size() < 2) - continue; - CBitcoinSecret vchSecret; - if (vchSecret.SetString(vstr[0])) { - CKey key = vchSecret.GetKey(); - CPubKey pubkey = key.GetPubKey(); - assert(key.VerifyPubKey(pubkey)); - CKeyID keyid = pubkey.GetID(); - if (pwallet->HaveKey(keyid)) { - LogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid)); + pwallet->ShowProgress(_("Importing..."), 0); // show progress dialog in GUI + while (file.good()) { + pwallet->ShowProgress("", std::max(1, std::min(99, (int)(((double)file.tellg() / (double)nFilesize) * 100)))); + std::string line; + std::getline(file, line); + if (line.empty() || line[0] == '#') continue; - } - int64_t nTime = DecodeDumpTime(vstr[1]); - std::string strLabel; - bool fLabel = true; - for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) { - if (boost::algorithm::starts_with(vstr[nStr], "#")) - break; - if (vstr[nStr] == "change=1") - fLabel = false; - if (vstr[nStr] == "reserve=1") - fLabel = false; - if (boost::algorithm::starts_with(vstr[nStr], "label=")) { - strLabel = DecodeDumpString(vstr[nStr].substr(6)); - fLabel = true; - } - } - LogPrintf("Importing %s...\n", EncodeDestination(keyid)); - if (!pwallet->AddKeyPubKey(key, pubkey)) { - fGood = false; + + std::vector<std::string> vstr; + boost::split(vstr, line, boost::is_any_of(" ")); + if (vstr.size() < 2) continue; + CBitcoinSecret vchSecret; + if (vchSecret.SetString(vstr[0])) { + CKey key = vchSecret.GetKey(); + CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); + CKeyID keyid = pubkey.GetID(); + if (pwallet->HaveKey(keyid)) { + LogPrintf("Skipping import of %s (key already present)\n", EncodeDestination(keyid)); + continue; + } + int64_t nTime = DecodeDumpTime(vstr[1]); + std::string strLabel; + bool fLabel = true; + for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) { + if (boost::algorithm::starts_with(vstr[nStr], "#")) + break; + if (vstr[nStr] == "change=1") + fLabel = false; + if (vstr[nStr] == "reserve=1") + fLabel = false; + if (boost::algorithm::starts_with(vstr[nStr], "label=")) { + strLabel = DecodeDumpString(vstr[nStr].substr(6)); + fLabel = true; + } + } + LogPrintf("Importing %s...\n", EncodeDestination(keyid)); + if (!pwallet->AddKeyPubKey(key, pubkey)) { + fGood = false; + continue; + } + pwallet->mapKeyMetadata[keyid].nCreateTime = nTime; + if (fLabel) + pwallet->SetAddressBook(keyid, strLabel, "receive"); + nTimeBegin = std::min(nTimeBegin, nTime); + } else if(IsHex(vstr[0])) { + std::vector<unsigned char> vData(ParseHex(vstr[0])); + CScript script = CScript(vData.begin(), vData.end()); + if (pwallet->HaveCScript(script)) { + LogPrintf("Skipping import of %s (script already present)\n", vstr[0]); + continue; + } + if(!pwallet->AddCScript(script)) { + LogPrintf("Error importing script %s\n", vstr[0]); + fGood = false; + continue; + } + int64_t birth_time = DecodeDumpTime(vstr[1]); + if (birth_time > 0) { + pwallet->m_script_metadata[CScriptID(script)].nCreateTime = birth_time; + nTimeBegin = std::min(nTimeBegin, birth_time); + } } - pwallet->mapKeyMetadata[keyid].nCreateTime = nTime; - if (fLabel) - pwallet->SetAddressBook(keyid, strLabel, "receive"); - nTimeBegin = std::min(nTimeBegin, nTime); - } else if(IsHex(vstr[0])) { - std::vector<unsigned char> vData(ParseHex(vstr[0])); - CScript script = CScript(vData.begin(), vData.end()); - if (pwallet->HaveCScript(script)) { - LogPrintf("Skipping import of %s (script already present)\n", vstr[0]); - continue; - } - if(!pwallet->AddCScript(script)) { - LogPrintf("Error importing script %s\n", vstr[0]); - fGood = false; - continue; - } - int64_t birth_time = DecodeDumpTime(vstr[1]); - if (birth_time > 0) { - pwallet->m_script_metadata[CScriptID(script)].nCreateTime = birth_time; - nTimeBegin = std::min(nTimeBegin, birth_time); - } } + file.close(); + pwallet->ShowProgress("", 100); // hide progress dialog in GUI + pwallet->UpdateTimeFirstKey(nTimeBegin); } - file.close(); - pwallet->ShowProgress("", 100); // hide progress dialog in GUI - pwallet->UpdateTimeFirstKey(nTimeBegin); - pwallet->RescanFromTime(nTimeBegin, false /* update */); + pwallet->RescanFromTime(nTimeBegin, reserver, false /* update */); pwallet->MarkDirty(); if (!fGood) @@ -685,7 +711,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) file << strprintf("# mined on %s\n", EncodeDumpTime(chainActive.Tip()->GetBlockTime())); file << "\n"; - // add the base58check encoded extended master if the wallet uses HD + // add the base58check encoded extended master if the wallet uses HD CKeyID masterKeyID = pwallet->GetHDChain().masterKeyID; if (!masterKeyID.IsNull()) { @@ -1110,6 +1136,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) " {\n" " \"rescan\": <false>, (boolean, optional, default: true) Stating if should rescan the blockchain after all imports\n" " }\n" + "\nNote: This call can take minutes to complete if rescan is true, during that time, other rpc calls\n" + "may report that the imported keys, addresses or scripts exists but related transactions are still missing.\n" "\nExamples:\n" + HelpExampleCli("importmulti", "'[{ \"scriptPubKey\": { \"address\": \"<my address>\" }, \"timestamp\":1455191478 }, " "{ \"scriptPubKey\": { \"address\": \"<my 2nd address>\" }, \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") + @@ -1135,49 +1163,55 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } } - LOCK2(cs_main, pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); - - // Verify all timestamps are present before importing any keys. - const int64_t now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0; - for (const UniValue& data : requests.getValues()) { - GetImportTimestamp(data, now); + WalletRescanReserver reserver(pwallet); + if (fRescan && !reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } + int64_t now = 0; bool fRunScan = false; - const int64_t minimumTimestamp = 1; int64_t nLowestTimestamp = 0; - - if (fRescan && chainActive.Tip()) { - nLowestTimestamp = chainActive.Tip()->GetBlockTime(); - } else { - fRescan = false; - } - UniValue response(UniValue::VARR); + { + LOCK2(cs_main, pwallet->cs_wallet); + EnsureWalletIsUnlocked(pwallet); - for (const UniValue& data : requests.getValues()) { - const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp); - const UniValue result = ProcessImport(pwallet, data, timestamp); - response.push_back(result); - - if (!fRescan) { - continue; + // Verify all timestamps are present before importing any keys. + now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0; + for (const UniValue& data : requests.getValues()) { + GetImportTimestamp(data, now); } - // If at least one request was successful then allow rescan. - if (result["success"].get_bool()) { - fRunScan = true; + const int64_t minimumTimestamp = 1; + + if (fRescan && chainActive.Tip()) { + nLowestTimestamp = chainActive.Tip()->GetBlockTime(); + } else { + fRescan = false; } - // Get the lowest timestamp. - if (timestamp < nLowestTimestamp) { - nLowestTimestamp = timestamp; + for (const UniValue& data : requests.getValues()) { + const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp); + const UniValue result = ProcessImport(pwallet, data, timestamp); + response.push_back(result); + + if (!fRescan) { + continue; + } + + // If at least one request was successful then allow rescan. + if (result["success"].get_bool()) { + fRunScan = true; + } + + // Get the lowest timestamp. + if (timestamp < nLowestTimestamp) { + nLowestTimestamp = timestamp; + } } } - if (fRescan && fRunScan && requests.size()) { - int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, true /* update */); + int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */); pwallet->ReacceptWalletTransactions(); if (scannedTime > nLowestTimestamp) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ad0ebcce22..6849671dcf 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -18,6 +18,7 @@ #include <rpc/mining.h> #include <rpc/safemode.h> #include <rpc/server.h> +#include <rpc/util.h> #include <script/sign.h> #include <timedata.h> #include <util.h> @@ -256,9 +257,9 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); } - OutputType output_type = g_change_type; + OutputType output_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type; if (!request.params[0].isNull()) { - output_type = ParseOutputType(request.params[0].get_str(), g_change_type); + output_type = ParseOutputType(request.params[0].get_str(), output_type); if (output_type == OUTPUT_TYPE_NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); } @@ -1161,9 +1162,6 @@ UniValue sendmany(const JSONRPCRequest& request) return wtx.GetHash().GetHex(); } -// Defined in rpc/misc.cpp -extern CScript _createmultisig_redeemScript(CWallet * const pwallet, const UniValue& params); - UniValue addmultisigaddress(const JSONRPCRequest& request) { CWallet * const pwallet = GetWalletForJSONRPCRequest(request); @@ -1181,16 +1179,22 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) "If 'account' is specified (DEPRECATED), assign address to that account.\n" "\nArguments:\n" - "1. nrequired (numeric, required) The number of required signatures out of the n keys or addresses.\n" - "2. \"keys\" (string, required) A json array of bitcoin addresses or hex-encoded public keys\n" + "1. nrequired (numeric, required) The number of required signatures out of the n keys or addresses.\n" + "2. \"keys\" (string, required) A json array of bitcoin addresses or hex-encoded public keys\n" " [\n" - " \"address\" (string) bitcoin address or hex-encoded public key\n" + " \"address\" (string) bitcoin address or hex-encoded public key\n" " ...,\n" " ]\n" - "3. \"account\" (string, optional) DEPRECATED. An account to assign the addresses to.\n" + "3. \"account\" (string, optional) DEPRECATED. An account to assign the addresses to.\n" "\nResult:\n" - "\"address\" (string) A bitcoin address associated with the keys.\n" + "{\n" + " \"address\":\"multisigaddress\", (string) The value of the new multisig address.\n" + " \"redeemScript\":\"script\" (string) The string value of the hex-encoded redemption script.\n" + "}\n" + "\nResult (DEPRECATED. To see this result in v0.16 instead, please start bitcoind with -deprecatedrpc=addmultisigaddress).\n" + " clients should transition to the new output api before upgrading to v0.17.\n" + "\"address\" (string) A bitcoin address associated with the keys.\n" "\nExamples:\n" "\nAdd a multisig address from 2 addresses\n" @@ -1207,14 +1211,34 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) if (!request.params[2].isNull()) strAccount = AccountFromValue(request.params[2]); + int required = request.params[0].get_int(); + + // Get the public keys + const UniValue& keys_or_addrs = request.params[1].get_array(); + std::vector<CPubKey> pubkeys; + for (unsigned int i = 0; i < keys_or_addrs.size(); ++i) { + if (IsHex(keys_or_addrs[i].get_str()) && (keys_or_addrs[i].get_str().length() == 66 || keys_or_addrs[i].get_str().length() == 130)) { + pubkeys.push_back(HexToPubKey(keys_or_addrs[i].get_str())); + } else { + pubkeys.push_back(AddrToPubKey(pwallet, keys_or_addrs[i].get_str())); + } + } + // Construct using pay-to-script-hash: - CScript inner = _createmultisig_redeemScript(pwallet, request.params); + CScript inner = CreateMultisigRedeemscript(required, pubkeys); pwallet->AddCScript(inner); - CTxDestination dest = pwallet->AddAndGetDestinationForScript(inner, g_address_type); - pwallet->SetAddressBook(dest, strAccount, "send"); - return EncodeDestination(dest); + + // Return old style interface + if (IsDeprecatedRPCEnabled("addmultisigaddress")) { + return EncodeDestination(dest); + } + + UniValue result(UniValue::VOBJ); + result.pushKV("address", EncodeDestination(dest)); + result.pushKV("redeemScript", HexStr(inner.begin(), inner.end())); + return result; } class Witnessifier : public boost::static_visitor<bool> @@ -3416,30 +3440,41 @@ UniValue rescanblockchain(const JSONRPCRequest& request) ); } - LOCK2(cs_main, pwallet->cs_wallet); + WalletRescanReserver reserver(pwallet); + if (!reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); + } - CBlockIndex *pindexStart = chainActive.Genesis(); + CBlockIndex *pindexStart = nullptr; CBlockIndex *pindexStop = nullptr; - if (!request.params[0].isNull()) { - pindexStart = chainActive[request.params[0].get_int()]; - if (!pindexStart) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid start_height"); - } - } + CBlockIndex *pChainTip = nullptr; + { + LOCK(cs_main); + pindexStart = chainActive.Genesis(); + pChainTip = chainActive.Tip(); - if (!request.params[1].isNull()) { - pindexStop = chainActive[request.params[1].get_int()]; - if (!pindexStop) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); + if (!request.params[0].isNull()) { + pindexStart = chainActive[request.params[0].get_int()]; + if (!pindexStart) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid start_height"); + } } - else if (pindexStop->nHeight < pindexStart->nHeight) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater then start_height"); + + if (!request.params[1].isNull()) { + pindexStop = chainActive[request.params[1].get_int()]; + if (!pindexStop) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); + } + else if (pindexStop->nHeight < pindexStart->nHeight) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater then start_height"); + } } } // We can't rescan beyond non-pruned blocks, stop and throw an error if (fPruneMode) { - CBlockIndex *block = pindexStop ? pindexStop : chainActive.Tip(); + LOCK(cs_main); + CBlockIndex *block = pindexStop ? pindexStop : pChainTip; while (block && block->nHeight >= pindexStart->nHeight) { if (!(block->nStatus & BLOCK_HAVE_DATA)) { throw JSONRPCError(RPC_MISC_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height."); @@ -3448,18 +3483,17 @@ UniValue rescanblockchain(const JSONRPCRequest& request) } } - CBlockIndex *stopBlock = pwallet->ScanForWalletTransactions(pindexStart, pindexStop, true); + CBlockIndex *stopBlock = pwallet->ScanForWalletTransactions(pindexStart, pindexStop, reserver, true); if (!stopBlock) { if (pwallet->IsAbortingRescan()) { throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted."); } // if we got a nullptr returned, ScanForWalletTransactions did rescan up to the requested stopindex - stopBlock = pindexStop ? pindexStop : chainActive.Tip(); + stopBlock = pindexStop ? pindexStop : pChainTip; } else { throw JSONRPCError(RPC_MISC_ERROR, "Rescan failed. Potentially corrupted data files."); } - UniValue response(UniValue::VOBJ); response.pushKV("start_height", pindexStart->nHeight); response.pushKV("stop_height", stopBlock->nHeight); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7b3c283f37..7e0881afd7 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -384,7 +384,9 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { CWallet wallet; AddKey(wallet, coinbaseKey); - BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip, nullptr)); + WalletRescanReserver reserver(&wallet); + reserver.reserve(); + BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip, nullptr, reserver)); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN); } @@ -397,7 +399,9 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { CWallet wallet; AddKey(wallet, coinbaseKey); - BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip, nullptr)); + WalletRescanReserver reserver(&wallet); + reserver.reserve(); + BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip, nullptr, reserver)); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN); } @@ -608,7 +612,9 @@ public: bool firstRun; wallet->LoadWallet(firstRun); AddKey(*wallet, coinbaseKey); - wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr); + WalletRescanReserver reserver(wallet.get()); + reserver.reserve(); + wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver); } ~ListCoinsTestingSetup() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index da721ea008..7fb26900e1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1612,19 +1612,20 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived, * @return Earliest timestamp that could be successfully scanned from. Timestamp * returned will be higher than startTime if relevant blocks could not be read. */ -int64_t CWallet::RescanFromTime(int64_t startTime, bool update) +int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update) { - AssertLockHeld(cs_main); - AssertLockHeld(cs_wallet); - // Find starting block. May be null if nCreateTime is greater than the // highest blockchain timestamp, in which case there is nothing that needs // to be scanned. - CBlockIndex* const startBlock = chainActive.FindEarliestAtLeast(startTime - TIMESTAMP_WINDOW); - LogPrintf("%s: Rescanning last %i blocks\n", __func__, startBlock ? chainActive.Height() - startBlock->nHeight + 1 : 0); + CBlockIndex* startBlock = nullptr; + { + LOCK(cs_main); + startBlock = chainActive.FindEarliestAtLeast(startTime - TIMESTAMP_WINDOW); + LogPrintf("%s: Rescanning last %i blocks\n", __func__, startBlock ? chainActive.Height() - startBlock->nHeight + 1 : 0); + } if (startBlock) { - const CBlockIndex* const failedBlock = ScanForWalletTransactions(startBlock, nullptr, update); + const CBlockIndex* const failedBlock = ScanForWalletTransactions(startBlock, nullptr, reserver, update); if (failedBlock) { return failedBlock->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1; } @@ -1643,12 +1644,17 @@ int64_t CWallet::RescanFromTime(int64_t startTime, bool update) * * If pindexStop is not a nullptr, the scan will stop at the block-index * defined by pindexStop + * + * Caller needs to make sure pindexStop (and the optional pindexStart) are on + * the main chain after to the addition of any new keys you want to detect + * transactions for. */ -CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, bool fUpdate) +CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, const WalletRescanReserver &reserver, bool fUpdate) { int64_t nNow = GetTime(); const CChainParams& chainParams = Params(); + assert(reserver.isReserved()); if (pindexStop) { assert(pindexStop->nHeight >= pindexStart->nHeight); } @@ -1656,24 +1662,42 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock CBlockIndex* pindex = pindexStart; CBlockIndex* ret = nullptr; { - LOCK2(cs_main, cs_wallet); fAbortRescan = false; - fScanningWallet = true; - ShowProgress(_("Rescanning..."), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup - double dProgressStart = GuessVerificationProgress(chainParams.TxData(), pindex); - double dProgressTip = GuessVerificationProgress(chainParams.TxData(), chainActive.Tip()); + CBlockIndex* tip = nullptr; + double dProgressStart; + double dProgressTip; + { + LOCK(cs_main); + tip = chainActive.Tip(); + dProgressStart = GuessVerificationProgress(chainParams.TxData(), pindex); + dProgressTip = GuessVerificationProgress(chainParams.TxData(), tip); + } while (pindex && !fAbortRescan) { - if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) - ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((GuessVerificationProgress(chainParams.TxData(), pindex) - dProgressStart) / (dProgressTip - dProgressStart) * 100)))); + if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) { + double gvp = 0; + { + LOCK(cs_main); + gvp = GuessVerificationProgress(chainParams.TxData(), pindex); + } + ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((gvp - dProgressStart) / (dProgressTip - dProgressStart) * 100)))); + } if (GetTime() >= nNow + 60) { nNow = GetTime(); + LOCK(cs_main); LogPrintf("Still rescanning. At block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex)); } CBlock block; if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { + LOCK2(cs_main, cs_wallet); + if (pindex && !chainActive.Contains(pindex)) { + // Abort scan if current block is no longer active, to prevent + // marking transactions as coming from the wrong block. + ret = pindex; + break; + } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate); } @@ -1683,14 +1707,20 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock if (pindex == pindexStop) { break; } - pindex = chainActive.Next(pindex); + { + LOCK(cs_main); + pindex = chainActive.Next(pindex); + if (tip != chainActive.Tip()) { + tip = chainActive.Tip(); + // in case the tip has changed, update progress max + dProgressTip = GuessVerificationProgress(chainParams.TxData(), tip); + } + } } if (pindex && fAbortRescan) { LogPrintf("Rescan aborted at block %d. Progress=%f\n", pindex->nHeight, GuessVerificationProgress(chainParams.TxData(), pindex)); } ShowProgress(_("Rescanning..."), 100); // hide progress dialog in GUI - - fScanningWallet = false; } return ret; } @@ -2644,6 +2674,34 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC return true; } +OutputType CWallet::TransactionChangeType(const std::vector<CRecipient>& vecSend) +{ + // If -changetype is specified, always use that change type. + if (g_change_type != OUTPUT_TYPE_NONE) { + return g_change_type; + } + + // if g_address_type is legacy, use legacy address as change (even + // if some of the outputs are P2WPKH or P2WSH). + if (g_address_type == OUTPUT_TYPE_LEGACY) { + return OUTPUT_TYPE_LEGACY; + } + + // if any destination is P2WPKH or P2WSH, use P2WPKH for the change + // output. + for (const auto& recipient : vecSend) { + // Check if any destination contains a witness program: + int witnessversion = 0; + std::vector<unsigned char> witnessprogram; + if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) { + return OUTPUT_TYPE_BECH32; + } + } + + // else use g_address_type for change + return g_address_type; +} + bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign) { @@ -2739,8 +2797,10 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT return false; } - LearnRelatedScripts(vchPubKey, g_change_type); - scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type)); + const OutputType change_type = TransactionChangeType(vecSend); + + LearnRelatedScripts(vchPubKey, change_type); + scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type)); } CTxOut change_prototype_txout(0, scriptChange); size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); @@ -4001,7 +4061,14 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) } nStart = GetTimeMillis(); - walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, true); + { + WalletRescanReserver reserver(walletInstance); + if (!reserver.reserve()) { + InitError(_("Failed to rescan the wallet during initialization")); + return nullptr; + } + walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, reserver, true); + } LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart); walletInstance->SetBestChain(chainActive.GetLocator()); walletInstance->dbw->IncrementUpdateCounter(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8acbade65c..e8f536634e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -659,6 +659,7 @@ private: }; +class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime /** * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. @@ -668,7 +669,10 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface private: static std::atomic<bool> fFlushScheduled; std::atomic<bool> fAbortRescan; - std::atomic<bool> fScanningWallet; + std::atomic<bool> fScanningWallet; //controlled by WalletRescanReserver + std::mutex mutexScanning; + friend class WalletRescanReserver; + /** * Select a set of coins such that nValueRet >= nTargetValue and at least @@ -945,8 +949,8 @@ public: void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex *pindex, const std::vector<CTransactionRef>& vtxConflicted) override; void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) override; bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate); - int64_t RescanFromTime(int64_t startTime, bool update); - CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, bool fUpdate = false); + int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); + CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, const WalletRescanReserver& reserver, bool fUpdate = false); void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override; @@ -961,6 +965,8 @@ public: CAmount GetLegacyBalance(const isminefilter& filter, int minDepth, const std::string* account) const; CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const; + OutputType TransactionChangeType(const std::vector<CRecipient>& vecSend); + /** * Insert additional inputs into the transaction by * calling CreateTransaction(); @@ -1263,4 +1269,39 @@ CTxDestination GetDestinationForKey(const CPubKey& key, OutputType); /** Get all destinations (potentially) supported by the wallet for the given key. */ std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key); +/** RAII object to check and reserve a wallet rescan */ +class WalletRescanReserver +{ +private: + CWalletRef m_wallet; + bool m_could_reserve; +public: + explicit WalletRescanReserver(CWalletRef w) : m_wallet(w), m_could_reserve(false) {} + + bool reserve() + { + assert(!m_could_reserve); + std::lock_guard<std::mutex> lock(m_wallet->mutexScanning); + if (m_wallet->fScanningWallet) { + return false; + } + m_wallet->fScanningWallet = true; + m_could_reserve = true; + return true; + } + + bool isReserved() const + { + return (m_could_reserve && m_wallet->fScanningWallet); + } + + ~WalletRescanReserver() + { + std::lock_guard<std::mutex> lock(m_wallet->mutexScanning); + if (m_could_reserve) { + m_wallet->fScanningWallet = false; + } + } +}; + #endif // BITCOIN_WALLET_WALLET_H diff --git a/test/functional/address_types.py b/test/functional/address_types.py index cb119c04b0..38a3425214 100755 --- a/test/functional/address_types.py +++ b/test/functional/address_types.py @@ -4,16 +4,24 @@ # 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: +There are 5 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 uses a p2sh/segwit addresses for change -node4 exists to generate new blocks. +node5 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: +## Multisig address test + +Test that adding a multisig address with: + - an uncompressed pubkey always gives a legacy address + - only compressed pubkeys gives the an `-addresstype` address + +## Sending to address types test + +A series of tests, iterating over node0-node4. 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 @@ -21,21 +29,51 @@ of the test, one node sends: - 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.""" +multisig addresses. + +Repeat test, but with explicit address_type parameters passed to getnewaddress +and getrawchangeaddress: + - node0 and node3 send to p2sh. + - node1 sends to bech32. + - node2 sends to legacy. + +As every node sends coins after receiving, this also +verifies that spending coins sent to all these address types works. + +## Change type test + +Test that the nodes generate the correct change address type: + - node0 always uses a legacy change address. + - node1 uses a bech32 addresses for change if any destination address is bech32. + - node2 always uses a bech32 address for change + - node3 always uses a bech32 address for change + - node4 always uses p2sh/segwit output for change. +""" 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 +from test_framework.util import ( + assert_equal, + assert_greater_than, + assert_raises_rpc_error, + 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"], []] + self.num_nodes = 6 + self.extra_args = [ + ["-addresstype=legacy"], + ["-addresstype=p2sh-segwit"], + ["-addresstype=p2sh-segwit", "-changetype=bech32"], + ["-addresstype=bech32"], + ["-changetype=p2sh-segwit"], + [] + ] def setup_network(self): self.setup_nodes() @@ -104,10 +142,26 @@ class AddressTypeTest(BitcoinTestFramework): # Unknown type assert(False) + def test_change_output_type(self, node_sender, destinations, expected_type): + txid = self.nodes[node_sender].sendmany(fromaccount="", amounts=dict.fromkeys(destinations, 0.001)) + raw_tx = self.nodes[node_sender].getrawtransaction(txid) + tx = self.nodes[node_sender].decoderawtransaction(raw_tx) + + # Make sure the transaction has change: + assert_equal(len(tx["vout"]), len(destinations) + 1) + + # Make sure the destinations are included, and remove them: + output_addresses = [vout['scriptPubKey']['addresses'][0] for vout in tx["vout"]] + change_addresses = [d for d in output_addresses if d not in destinations] + assert_equal(len(change_addresses), 1) + + self.log.debug("Check if change address " + change_addresses[0] + " is " + expected_type) + self.test_address(node_sender, change_addresses[0], multisig=False, typ=expected_type) + def run_test(self): - # Mine 101 blocks on node4 to bring nodes out of IBD and make sure that + # Mine 101 blocks on node5 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) + self.nodes[5].generate(101) sync_blocks(self.nodes) uncompressed_1 = "0496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858ee" @@ -117,14 +171,14 @@ class AddressTypeTest(BitcoinTestFramework): # 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') + self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, uncompressed_2])['address'], True, 'legacy') + self.test_address(node, self.nodes[node].addmultisigaddress(2, [compressed_1, uncompressed_2])['address'], True, 'legacy') + self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, compressed_2])['address'], 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') + self.test_address(0, self.nodes[0].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'legacy') + self.test_address(1, self.nodes[1].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'p2sh-segwit') + self.test_address(2, self.nodes[2].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'p2sh-segwit') + self.test_address(3, self.nodes[3].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'bech32') for explicit_type, multisig, from_node in itertools.product([False, True], [False, True], range(4)): address_type = None @@ -155,7 +209,7 @@ class AddressTypeTest(BitcoinTestFramework): else: addr1 = self.nodes[to_node].getnewaddress() addr2 = self.nodes[to_node].getnewaddress() - address = self.nodes[to_node].addmultisigaddress(2, [addr1, addr2]) + address = self.nodes[to_node].addmultisigaddress(2, [addr1, addr2])['address'] # Do some sanity checking on the created address if address_type is not None: @@ -182,8 +236,8 @@ class AddressTypeTest(BitcoinTestFramework): 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) + # node5 collects fee and block subsidy to keep accounting simple + self.nodes[5].generate(1) sync_blocks(self.nodes) new_balances = self.get_balances() @@ -195,5 +249,46 @@ class AddressTypeTest(BitcoinTestFramework): to_node %= 4 assert_equal(new_balances[to_node], old_balances[to_node] + to_send * 10 * (2 + n)) + # Get one p2sh/segwit address from node2 and two bech32 addresses from node3: + to_address_p2sh = self.nodes[2].getnewaddress() + to_address_bech32_1 = self.nodes[3].getnewaddress() + to_address_bech32_2 = self.nodes[3].getnewaddress() + + # Fund node 4: + self.nodes[5].sendtoaddress(self.nodes[4].getnewaddress(), Decimal("1")) + self.nodes[5].generate(1) + sync_blocks(self.nodes) + assert_equal(self.nodes[4].getbalance(), 1) + + self.log.info("Nodes with addresstype=legacy never use a P2WPKH change output") + self.test_change_output_type(0, [to_address_bech32_1], 'legacy') + + self.log.info("Nodes with addresstype=p2sh-segwit only use a P2WPKH change output if any destination address is bech32:") + self.test_change_output_type(1, [to_address_p2sh], 'p2sh-segwit') + self.test_change_output_type(1, [to_address_bech32_1], 'bech32') + self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32') + self.test_change_output_type(1, [to_address_bech32_1, to_address_bech32_2], 'bech32') + + self.log.info("Nodes with change_type=bech32 always use a P2WPKH change output:") + self.test_change_output_type(2, [to_address_bech32_1], 'bech32') + self.test_change_output_type(2, [to_address_p2sh], 'bech32') + + self.log.info("Nodes with addresstype=bech32 always use a P2WPKH change output (unless changetype is set otherwise):") + self.test_change_output_type(3, [to_address_bech32_1], 'bech32') + self.test_change_output_type(3, [to_address_p2sh], 'bech32') + + self.log.info('getrawchangeaddress defaults to addresstype if -changetype is not set and argument is absent') + self.test_address(3, self.nodes[3].getrawchangeaddress(), multisig=False, typ='bech32') + + self.log.info('getrawchangeaddress fails with invalid changetype argument') + assert_raises_rpc_error(-5, "Unknown address type 'bech23'", self.nodes[3].getrawchangeaddress, 'bech23') + + self.log.info("Nodes with changetype=p2sh-segwit never use a P2WPKH change output") + self.test_change_output_type(4, [to_address_bech32_1], 'p2sh-segwit') + self.test_address(4, self.nodes[4].getrawchangeaddress(), multisig=False, typ='p2sh-segwit') + self.log.info("Except for getrawchangeaddress if specified:") + self.test_address(4, self.nodes[4].getrawchangeaddress(), multisig=False, typ='p2sh-segwit') + self.test_address(4, self.nodes[4].getrawchangeaddress('bech32'), multisig=False, typ='bech32') + if __name__ == '__main__': AddressTypeTest().main() diff --git a/test/functional/bitcoin_cli.py b/test/functional/bitcoin_cli.py index d1cd3b3620..d8c80ab34f 100755 --- a/test/functional/bitcoin_cli.py +++ b/test/functional/bitcoin_cli.py @@ -39,7 +39,7 @@ class TestBitcoinCli(BitcoinTestFramework): assert_raises_process_error(1, "-getinfo takes no arguments", self.nodes[0].cli('-getinfo').help) self.log.info("Compare responses from `bitcoin-cli -getinfo` and the RPCs data is retrieved from.") - cli_get_info = self.nodes[0].cli().send_cli('-getinfo') + cli_get_info = self.nodes[0].cli('-getinfo').send_cli() wallet_info = self.nodes[0].getwalletinfo() network_info = self.nodes[0].getnetworkinfo() blockchain_info = self.nodes[0].getblockchaininfo() diff --git a/test/functional/deprecated_rpc.py b/test/functional/deprecated_rpc.py index 19fd24edb9..d6f25158ef 100755 --- a/test/functional/deprecated_rpc.py +++ b/test/functional/deprecated_rpc.py @@ -10,7 +10,7 @@ class DeprecatedRpcTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True - self.extra_args = [[], ["-deprecatedrpc=estimatefee"]] + self.extra_args = [[], ["-deprecatedrpc=estimatefee", "-deprecatedrpc=createmultisig"]] def run_test(self): self.log.info("estimatefee: Shows deprecated message") @@ -19,5 +19,9 @@ class DeprecatedRpcTest(BitcoinTestFramework): self.log.info("Using -deprecatedrpc=estimatefee bypasses the error") self.nodes[1].estimatefee(1) + self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses") + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()]) + self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()]) + if __name__ == '__main__': DeprecatedRpcTest().main() diff --git a/test/functional/fundrawtransaction.py b/test/functional/fundrawtransaction.py index 4739e73c39..b3d6549229 100755 --- a/test/functional/fundrawtransaction.py +++ b/test/functional/fundrawtransaction.py @@ -358,7 +358,7 @@ class RawTransactionsTest(BitcoinTestFramework): addr1Obj = self.nodes[1].validateaddress(addr1) addr2Obj = self.nodes[1].validateaddress(addr2) - mSigObj = self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) + mSigObj = self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] inputs = [] outputs = {mSigObj:1.1} @@ -391,7 +391,7 @@ class RawTransactionsTest(BitcoinTestFramework): addr4Obj = self.nodes[1].validateaddress(addr4) addr5Obj = self.nodes[1].validateaddress(addr5) - mSigObj = self.nodes[1].addmultisigaddress(4, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey'], addr4Obj['pubkey'], addr5Obj['pubkey']]) + mSigObj = self.nodes[1].addmultisigaddress(4, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey'], addr4Obj['pubkey'], addr5Obj['pubkey']])['address'] inputs = [] outputs = {mSigObj:1.1} @@ -418,7 +418,7 @@ class RawTransactionsTest(BitcoinTestFramework): addr1Obj = self.nodes[2].validateaddress(addr1) addr2Obj = self.nodes[2].validateaddress(addr2) - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] # send 1.2 BTC to msig addr diff --git a/test/functional/importmulti.py b/test/functional/importmulti.py index ab4ca48e48..be9be83839 100755 --- a/test/functional/importmulti.py +++ b/test/functional/importmulti.py @@ -228,7 +228,7 @@ class ImportMultiTest (BitcoinTestFramework): sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) - multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']]) + multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']]) self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) @@ -255,7 +255,7 @@ class ImportMultiTest (BitcoinTestFramework): sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) - multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']]) + multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']]) self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) @@ -282,7 +282,7 @@ class ImportMultiTest (BitcoinTestFramework): sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) - multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']]) + multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']]) self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) @@ -309,7 +309,7 @@ class ImportMultiTest (BitcoinTestFramework): sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) - multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']]) + multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']]) self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) diff --git a/test/functional/listtransactions.py b/test/functional/listtransactions.py index cbed065928..ba71ac0967 100755 --- a/test/functional/listtransactions.py +++ b/test/functional/listtransactions.py @@ -81,7 +81,8 @@ class ListTransactionsTest(BitcoinTestFramework): {"category":"receive","amount":Decimal("0.44")}, {"txid":txid, "account" : "toself"} ) - multisig = self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()]) + pubkey = self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['pubkey'] + multisig = self.nodes[1].createmultisig(1, [pubkey]) self.nodes[0].importaddress(multisig["redeemScript"], "watchonly", False, True) txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1) self.nodes[1].generate(1) diff --git a/test/functional/nulldummy.py b/test/functional/nulldummy.py index cbdcfdcf5f..fac620bc8d 100755 --- a/test/functional/nulldummy.py +++ b/test/functional/nulldummy.py @@ -46,7 +46,7 @@ class NULLDUMMYTest(BitcoinTestFramework): def run_test(self): self.address = self.nodes[0].getnewaddress() - self.ms_address = self.nodes[0].addmultisigaddress(1,[self.address]) + self.ms_address = self.nodes[0].addmultisigaddress(1,[self.address])['address'] self.wit_address = self.nodes[0].addwitnessaddress(self.address) self.wit_ms_address = self.nodes[0].addwitnessaddress(self.ms_address) diff --git a/test/functional/rawtransactions.py b/test/functional/rawtransactions.py index fe749adb49..d39d86b310 100755 --- a/test/functional/rawtransactions.py +++ b/test/functional/rawtransactions.py @@ -145,7 +145,12 @@ class RawTransactionsTest(BitcoinTestFramework): addr1Obj = self.nodes[2].validateaddress(addr1) addr2Obj = self.nodes[2].validateaddress(addr2) - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) + # Tests for createmultisig and addmultisigaddress + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) + self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here. + + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] #use balance deltas instead of absolute values bal = self.nodes[2].getbalance() @@ -168,7 +173,7 @@ class RawTransactionsTest(BitcoinTestFramework): addr2Obj = self.nodes[2].validateaddress(addr2) addr3Obj = self.nodes[2].validateaddress(addr3) - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']]) + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address'] txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) decTx = self.nodes[0].gettransaction(txId) @@ -213,8 +218,8 @@ class RawTransactionsTest(BitcoinTestFramework): addr1Obj = self.nodes[1].validateaddress(addr1) addr2Obj = self.nodes[2].validateaddress(addr2) - self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) + self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] mSigObjValid = self.nodes[2].validateaddress(mSigObj) txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) diff --git a/test/functional/segwit.py b/test/functional/segwit.py index 67cca5f1e7..931ebdea39 100755 --- a/test/functional/segwit.py +++ b/test/functional/segwit.py @@ -95,7 +95,7 @@ class SegWitTest(BitcoinTestFramework): for i in range(3): newaddress = self.nodes[i].getnewaddress() self.pubkey.append(self.nodes[i].validateaddress(newaddress)["pubkey"]) - multiaddress = self.nodes[i].addmultisigaddress(1, [self.pubkey[-1]]) + multiaddress = self.nodes[i].addmultisigaddress(1, [self.pubkey[-1]])['address'] multiscript = CScript([OP_1, hex_str_to_bytes(self.pubkey[-1]), OP_1, OP_CHECKMULTISIG]) p2sh_addr = self.nodes[i].addwitnessaddress(newaddress) bip173_addr = self.nodes[i].addwitnessaddress(newaddress, False) @@ -290,19 +290,19 @@ class SegWitTest(BitcoinTestFramework): 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]])) - compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]])) - uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], uncompressed_solvable_address[0]])) - compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]])) - compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], compressed_solvable_address[1]])) + uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]])['address']) + uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]])['address']) + compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]])['address']) + uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], uncompressed_solvable_address[0]])['address']) + compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]])['address']) + compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], compressed_solvable_address[1]])['address']) unknown_address = ["mtKKyoHabkk6e4ppT7NaM7THqPUt7AzPrT", "2NDP3jLWAFT8NDAiUa9qiE6oBt2awmMq7Dx"] # Test multisig_without_privkey # We have 2 public keys without private keys, use addmultisigaddress to add to wallet. # Money sent to P2SH of multisig of this should only be seen after importaddress with the BASE58 P2SH address. - multisig_without_privkey_address = self.nodes[0].addmultisigaddress(2, [pubkeys[3], pubkeys[4]]) + multisig_without_privkey_address = self.nodes[0].addmultisigaddress(2, [pubkeys[3], pubkeys[4]])['address'] script = CScript([OP_2, hex_str_to_bytes(pubkeys[3]), hex_str_to_bytes(pubkeys[4]), OP_2, OP_CHECKMULTISIG]) solvable_after_importaddress.append(CScript([OP_HASH160, hash160(script), OP_EQUAL])) @@ -463,11 +463,11 @@ class SegWitTest(BitcoinTestFramework): 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]])) - compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]])) - uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], uncompressed_solvable_address[0]])) - compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]])) + uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]])['address']) + uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]])['address']) + compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]])['address']) + uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], uncompressed_solvable_address[0]])['address']) + compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]])['address']) premature_witaddress = [] diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 7c784d8840..1054e6d028 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -213,16 +213,16 @@ class TestNodeCLI(): """Interface to bitcoin-cli for an individual node""" def __init__(self, binary, datadir): - self.args = [] + self.options = [] self.binary = binary self.datadir = datadir self.input = None self.log = logging.getLogger('TestFramework.bitcoincli') - def __call__(self, *args, input=None): - # TestNodeCLI is callable with bitcoin-cli command-line args + def __call__(self, *options, input=None): + # TestNodeCLI is callable with bitcoin-cli command-line options cli = TestNodeCLI(self.binary, self.datadir) - cli.args = [str(arg) for arg in args] + cli.options = [str(o) for o in options] cli.input = input return cli @@ -238,16 +238,18 @@ class TestNodeCLI(): results.append(dict(error=e)) return results - def send_cli(self, command, *args, **kwargs): + def send_cli(self, command=None, *args, **kwargs): """Run bitcoin-cli command. Deserializes returned string as python object.""" pos_args = [str(arg) for arg in args] named_args = [str(key) + "=" + str(value) for (key, value) in kwargs.items()] assert not (pos_args and named_args), "Cannot use positional arguments and named arguments in the same bitcoin-cli call" - p_args = [self.binary, "-datadir=" + self.datadir] + self.args + p_args = [self.binary, "-datadir=" + self.datadir] + self.options if named_args: p_args += ["-named"] - p_args += [command] + pos_args + named_args + if command is not None: + p_args += [command] + p_args += pos_args + named_args self.log.debug("Running bitcoin-cli command: %s" % command) process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) cli_stdout, cli_stderr = process.communicate(input=self.input) diff --git a/test/functional/wallet-accounts.py b/test/functional/wallet-accounts.py index 287436877f..ecd1cfc82b 100755 --- a/test/functional/wallet-accounts.py +++ b/test/functional/wallet-accounts.py @@ -126,7 +126,7 @@ class WalletAccountsTest(BitcoinTestFramework): addresses = [] for x in range(10): addresses.append(node.getnewaddress()) - multisig_address = node.addmultisigaddress(5, addresses, account.name) + multisig_address = node.addmultisigaddress(5, addresses, account.name)['address'] account.add_address(multisig_address) account.verify(node) node.sendfrom("", multisig_address, 50) diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py index a1a4f25921..77f90ffb81 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -94,7 +94,7 @@ class WalletDumpTest(BitcoinTestFramework): # Test scripts dump by adding a P2SH witness and a 1-of-1 multisig address witness_addr = self.nodes[0].addwitnessaddress(addrs[0]["address"], True) - multisig_addr = self.nodes[0].addmultisigaddress(1, [addrs[1]["address"]]) + multisig_addr = self.nodes[0].addmultisigaddress(1, [addrs[1]["address"]])["address"] script_addrs = [witness_addr, multisig_addr] # dump unencrypted wallet |