diff options
Diffstat (limited to 'src/wallet')
| -rw-r--r-- | src/wallet/db.cpp | 41 | ||||
| -rw-r--r-- | src/wallet/db.h | 81 | ||||
| -rw-r--r-- | src/wallet/init.cpp | 8 | ||||
| -rw-r--r-- | src/wallet/rpcdump.cpp | 383 | ||||
| -rw-r--r-- | src/wallet/rpcwallet.cpp | 65 | ||||
| -rw-r--r-- | src/wallet/test/wallet_tests.cpp | 4 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 129 | ||||
| -rw-r--r-- | src/wallet/wallet.h | 17 | ||||
| -rw-r--r-- | src/wallet/walletdb.cpp | 20 |
9 files changed, 449 insertions, 299 deletions
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 98e2abbd18..a2e795056a 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -263,6 +263,45 @@ BerkeleyEnvironment::VerifyResult BerkeleyEnvironment::Verify(const std::string& return (fRecovered ? VerifyResult::RECOVER_OK : VerifyResult::RECOVER_FAIL); } +BerkeleyBatch::SafeDbt::SafeDbt() +{ + m_dbt.set_flags(DB_DBT_MALLOC); +} + +BerkeleyBatch::SafeDbt::SafeDbt(void* data, size_t size) + : m_dbt(data, size) +{ +} + +BerkeleyBatch::SafeDbt::~SafeDbt() +{ + if (m_dbt.get_data() != nullptr) { + // Clear memory, e.g. in case it was a private key + memory_cleanse(m_dbt.get_data(), m_dbt.get_size()); + // under DB_DBT_MALLOC, data is malloced by the Dbt, but must be + // freed by the caller. + // https://docs.oracle.com/cd/E17275_01/html/api_reference/C/dbt.html + if (m_dbt.get_flags() & DB_DBT_MALLOC) { + free(m_dbt.get_data()); + } + } +} + +const void* BerkeleyBatch::SafeDbt::get_data() const +{ + return m_dbt.get_data(); +} + +u_int32_t BerkeleyBatch::SafeDbt::get_size() const +{ + return m_dbt.get_size(); +} + +BerkeleyBatch::SafeDbt::operator Dbt*() +{ + return &m_dbt; +} + bool BerkeleyBatch::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) { std::string filename; @@ -338,7 +377,7 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, std::string& er BerkeleyEnvironment* env = GetWalletEnv(file_path, walletFile); fs::path walletDir = env->Directory(); - LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0)); + LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(nullptr, nullptr, nullptr)); LogPrintf("Using wallet %s\n", walletFile); // Wallet file must be a plain filename without a directory diff --git a/src/wallet/db.h b/src/wallet/db.h index e453d441d7..9dc373c89b 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -191,10 +191,29 @@ private: bool IsDummy() { return env == nullptr; } }; - /** RAII class that provides access to a Berkeley database */ class BerkeleyBatch { + /** RAII class that automatically cleanses its data on destruction */ + class SafeDbt final + { + Dbt m_dbt; + + public: + // construct Dbt with internally-managed data + SafeDbt(); + // construct Dbt with provided data + SafeDbt(void* data, size_t size); + ~SafeDbt(); + + // delegate to Dbt + const void* get_data() const; + u_int32_t get_size() const; + + // conversion operator to access the underlying Dbt + operator Dbt*(); + }; + protected: Db* pdb; std::string strFile; @@ -222,7 +241,6 @@ public: /* verifies the database file */ static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, BerkeleyEnvironment::recoverFunc_type recoverFunc); -public: template <typename K, typename T> bool Read(const K& key, T& value) { @@ -233,13 +251,11 @@ public: CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - Dbt datKey(ssKey.data(), ssKey.size()); + SafeDbt datKey(ssKey.data(), ssKey.size()); // Read - Dbt datValue; - datValue.set_flags(DB_DBT_MALLOC); - int ret = pdb->get(activeTxn, &datKey, &datValue, 0); - memory_cleanse(datKey.get_data(), datKey.get_size()); + SafeDbt datValue; + int ret = pdb->get(activeTxn, datKey, datValue, 0); bool success = false; if (datValue.get_data() != nullptr) { // Unserialize value @@ -250,10 +266,6 @@ public: } catch (const std::exception&) { // In this case success remains 'false' } - - // Clear and free memory - memory_cleanse(datValue.get_data(), datValue.get_size()); - free(datValue.get_data()); } return ret == 0 && success; } @@ -270,20 +282,16 @@ public: CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - Dbt datKey(ssKey.data(), ssKey.size()); + SafeDbt datKey(ssKey.data(), ssKey.size()); // Value CDataStream ssValue(SER_DISK, CLIENT_VERSION); ssValue.reserve(10000); ssValue << value; - Dbt datValue(ssValue.data(), ssValue.size()); + SafeDbt datValue(ssValue.data(), ssValue.size()); // Write - int ret = pdb->put(activeTxn, &datKey, &datValue, (fOverwrite ? 0 : DB_NOOVERWRITE)); - - // Clear memory in case it was a private key - memory_cleanse(datKey.get_data(), datKey.get_size()); - memory_cleanse(datValue.get_data(), datValue.get_size()); + int ret = pdb->put(activeTxn, datKey, datValue, (fOverwrite ? 0 : DB_NOOVERWRITE)); return (ret == 0); } @@ -299,13 +307,10 @@ public: CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - Dbt datKey(ssKey.data(), ssKey.size()); + SafeDbt datKey(ssKey.data(), ssKey.size()); // Erase - int ret = pdb->del(activeTxn, &datKey, 0); - - // Clear memory - memory_cleanse(datKey.get_data(), datKey.get_size()); + int ret = pdb->del(activeTxn, datKey, 0); return (ret == 0 || ret == DB_NOTFOUND); } @@ -319,13 +324,10 @@ public: CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - Dbt datKey(ssKey.data(), ssKey.size()); + SafeDbt datKey(ssKey.data(), ssKey.size()); // Exists - int ret = pdb->exists(activeTxn, &datKey, 0); - - // Clear memory - memory_cleanse(datKey.get_data(), datKey.get_size()); + int ret = pdb->exists(activeTxn, datKey, 0); return (ret == 0); } @@ -340,20 +342,12 @@ public: return pcursor; } - int ReadAtCursor(Dbc* pcursor, CDataStream& ssKey, CDataStream& ssValue, bool setRange = false) + int ReadAtCursor(Dbc* pcursor, CDataStream& ssKey, CDataStream& ssValue) { // Read at cursor - Dbt datKey; - unsigned int fFlags = DB_NEXT; - if (setRange) { - datKey.set_data(ssKey.data()); - datKey.set_size(ssKey.size()); - fFlags = DB_SET_RANGE; - } - Dbt datValue; - datKey.set_flags(DB_DBT_MALLOC); - datValue.set_flags(DB_DBT_MALLOC); - int ret = pcursor->get(&datKey, &datValue, fFlags); + SafeDbt datKey; + SafeDbt datValue; + int ret = pcursor->get(datKey, datValue, DB_NEXT); if (ret != 0) return ret; else if (datKey.get_data() == nullptr || datValue.get_data() == nullptr) @@ -366,16 +360,9 @@ public: ssValue.SetType(SER_DISK); ssValue.clear(); ssValue.write((char*)datValue.get_data(), datValue.get_size()); - - // Clear and free memory - memory_cleanse(datKey.get_data(), datKey.get_size()); - memory_cleanse(datValue.get_data(), datValue.get_size()); - free(datKey.get_data()); - free(datValue.get_data()); return 0; } -public: bool TxnBegin() { if (!pdb || activeTxn) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 14d811c6cd..87cd264c3d 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -242,7 +242,11 @@ void StopWallets() void UnloadWallets() { - for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { - RemoveWallet(pwallet); + auto wallets = GetWallets(); + while (!wallets.empty()) { + auto wallet = wallets.back(); + wallets.pop_back(); + RemoveWallet(wallet); + UnloadWallet(std::move(wallet)); } } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 647af3eb86..295dfcc63f 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -836,9 +836,98 @@ UniValue dumpwallet(const JSONRPCRequest& request) return reply; } +struct ImportData +{ + // Input data + std::unique_ptr<CScript> redeemscript; //!< Provided redeemScript; will be moved to `import_scripts` if relevant. + std::unique_ptr<CScript> witnessscript; //!< Provided witnessScript; will be moved to `import_scripts` if relevant. + + // Output data + std::set<CScript> import_scripts; + std::map<CKeyID, bool> used_keys; //!< Import these private keys if available (the value indicates whether if the key is required for solvability) +}; + +enum class ScriptContext +{ + TOP, //! Top-level scriptPubKey + P2SH, //! P2SH redeemScript + WITNESS_V0, //! P2WSH witnessScript +}; + +// Analyse the provided scriptPubKey, determining which keys and which redeem scripts from the ImportData struct are needed to spend it, and mark them as used. +// Returns an error string, or the empty string for success. +static std::string RecurseImportData(const CScript& script, ImportData& import_data, const ScriptContext script_ctx) +{ + // Use Solver to obtain script type and parsed pubkeys or hashes: + std::vector<std::vector<unsigned char>> solverdata; + txnouttype script_type = Solver(script, solverdata); + + switch (script_type) { + case TX_PUBKEY: { + CPubKey pubkey(solverdata[0].begin(), solverdata[0].end()); + import_data.used_keys.emplace(pubkey.GetID(), false); + return ""; + } + case TX_PUBKEYHASH: { + CKeyID id = CKeyID(uint160(solverdata[0])); + import_data.used_keys[id] = true; + return ""; + } + case TX_SCRIPTHASH: { + if (script_ctx == ScriptContext::P2SH) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside another P2SH"); + if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2SH inside a P2WSH"); + assert(script_ctx == ScriptContext::TOP); + CScriptID id = CScriptID(uint160(solverdata[0])); + auto subscript = std::move(import_data.redeemscript); // Remove redeemscript from import_data to check for superfluous script later. + if (!subscript) return "missing redeemscript"; + if (CScriptID(*subscript) != id) return "redeemScript does not match the scriptPubKey"; + import_data.import_scripts.emplace(*subscript); + return RecurseImportData(*subscript, import_data, ScriptContext::P2SH); + } + case TX_MULTISIG: { + for (size_t i = 1; i + 1< solverdata.size(); ++i) { + CPubKey pubkey(solverdata[i].begin(), solverdata[i].end()); + import_data.used_keys.emplace(pubkey.GetID(), false); + } + return ""; + } + case TX_WITNESS_V0_SCRIPTHASH: { + if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2WSH inside another P2WSH"); + uint256 fullid(solverdata[0]); + CScriptID id; + CRIPEMD160().Write(fullid.begin(), fullid.size()).Finalize(id.begin()); + auto subscript = std::move(import_data.witnessscript); // Remove redeemscript from import_data to check for superfluous script later. + if (!subscript) return "missing witnessscript"; + if (CScriptID(*subscript) != id) return "witnessScript does not match the scriptPubKey or redeemScript"; + if (script_ctx == ScriptContext::TOP) { + import_data.import_scripts.emplace(script); // Special rule for IsMine: native P2WSH requires the TOP script imported (see script/ismine.cpp) + } + import_data.import_scripts.emplace(*subscript); + return RecurseImportData(*subscript, import_data, ScriptContext::WITNESS_V0); + } + case TX_WITNESS_V0_KEYHASH: { + if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2WPKH inside P2WSH"); + CKeyID id = CKeyID(uint160(solverdata[0])); + import_data.used_keys[id] = true; + if (script_ctx == ScriptContext::TOP) { + import_data.import_scripts.emplace(script); // Special rule for IsMine: native P2WPKH requires the TOP script imported (see script/ismine.cpp) + } + return ""; + } + case TX_NULL_DATA: + return "unspendable script"; + case TX_NONSTANDARD: + case TX_WITNESS_UNKNOWN: + default: + return "unrecognized script"; + } +} static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + UniValue warnings(UniValue::VARR); + UniValue result(UniValue::VOBJ); + try { // First ensure scriptPubKey has either a script or JSON with "address" string const UniValue& scriptPubKey = data["scriptPubKey"]; @@ -860,18 +949,16 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con // Generate the script and destination for the scriptPubKey provided CScript script; CTxDestination dest; - if (!isScript) { dest = DecodeDestination(output); if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address \"" + output + "\""); } script = GetScriptForDestination(dest); } else { if (!IsHex(output)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid scriptPubKey"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid scriptPubKey \"" + output + "\""); } - std::vector<unsigned char> vData(ParseHex(output)); script = CScript(vData.begin(), vData.end()); if (!ExtractDestination(script, dest) && !internal) { @@ -879,203 +966,161 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con } } - // Watchonly and private keys - if (watchOnly && keys.size()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Watch-only addresses should not include private keys"); - } - - // Internal addresses should not have a label - if (internal && data.exists("label")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label"); - } - - // Force users to provide the witness script in its field rather than redeemscript - if (!strRedeemScript.empty() && script.IsPayToWitnessScriptHash()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "P2WSH addresses have an empty redeemscript. Please provide the witnessscript instead."); - } - - CScript scriptpubkey_script = script; - CTxDestination scriptpubkey_dest = dest; - bool allow_p2wpkh = true; - - // P2SH - if (!strRedeemScript.empty() && script.IsPayToScriptHash()) { - // Check the redeemScript is valid + // Parse all arguments + ImportData import_data; + if (strRedeemScript.size()) { if (!IsHex(strRedeemScript)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script: must be hex string"); - } - - // Import redeem script. - std::vector<unsigned char> vData(ParseHex(strRedeemScript)); - CScript redeemScript = CScript(vData.begin(), vData.end()); - CScriptID redeem_id(redeemScript); - - // Check that the redeemScript and scriptPubKey match - if (GetScriptForDestination(redeem_id) != script) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The redeemScript does not match the scriptPubKey"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script \"" + strRedeemScript + "\": must be hex string"); } - - pwallet->MarkDirty(); - - if (!pwallet->AddWatchOnly(redeemScript, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); - } - - if (!pwallet->HaveCScript(redeem_id) && !pwallet->AddCScript(redeemScript)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet"); - } - - // Now set script to the redeemScript so we parse the inner script as P2WSH or P2WPKH below - script = redeemScript; - ExtractDestination(script, dest); + auto parsed_redeemscript = ParseHex(strRedeemScript); + import_data.redeemscript = MakeUnique<CScript>(parsed_redeemscript.begin(), parsed_redeemscript.end()); } - - // (P2SH-)P2WSH - if (!witness_script_hex.empty() && script.IsPayToWitnessScriptHash()) { + if (witness_script_hex.size()) { if (!IsHex(witness_script_hex)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid witness script: must be hex string"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid witness script \"" + witness_script_hex + "\": must be hex string"); } - - // Generate the scripts - std::vector<unsigned char> witness_script_parsed(ParseHex(witness_script_hex)); - CScript witness_script = CScript(witness_script_parsed.begin(), witness_script_parsed.end()); - CScriptID witness_id(witness_script); - - // Check that the witnessScript and scriptPubKey match - if (GetScriptForDestination(WitnessV0ScriptHash(witness_script)) != script) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The witnessScript does not match the scriptPubKey or redeemScript"); + auto parsed_witnessscript = ParseHex(witness_script_hex); + import_data.witnessscript = MakeUnique<CScript>(parsed_witnessscript.begin(), parsed_witnessscript.end()); + } + std::map<CKeyID, CPubKey> pubkey_map; + for (size_t i = 0; i < pubKeys.size(); ++i) { + const auto& str = pubKeys[i].get_str(); + if (!IsHex(str)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" must be a hex string"); } - - // Add the witness script as watch only only if it is not for P2SH-P2WSH - if (!scriptpubkey_script.IsPayToScriptHash() && !pwallet->AddWatchOnly(witness_script, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); + auto parsed_pubkey = ParseHex(str); + CPubKey pubkey(parsed_pubkey.begin(), parsed_pubkey.end()); + if (!pubkey.IsFullyValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" is not a valid public key"); } - - if (!pwallet->HaveCScript(witness_id) && !pwallet->AddCScript(witness_script)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2wsh witnessScript to wallet"); + pubkey_map.emplace(pubkey.GetID(), pubkey); + } + std::map<CKeyID, CKey> privkey_map; + for (size_t i = 0; i < keys.size(); ++i) { + const auto& str = keys[i].get_str(); + CKey key = DecodeSecret(str); + if (!key.IsValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); } + CPubKey pubkey = key.GetPubKey(); + CKeyID id = pubkey.GetID(); + if (pubkey_map.count(id)) { + pubkey_map.erase(id); + } + privkey_map.emplace(id, key); + } - // Now set script to the witnessScript so we parse the inner script as P2PK or P2PKH below - script = witness_script; - ExtractDestination(script, dest); - allow_p2wpkh = false; // P2WPKH cannot be embedded in P2WSH + // Internal addresses should not have a label + if (internal && data.exists("label")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label"); } - // (P2SH-)P2PK/P2PKH/P2WPKH - if (dest.type() == typeid(CKeyID) || dest.type() == typeid(WitnessV0KeyHash)) { - if (!allow_p2wpkh && dest.type() == typeid(WitnessV0KeyHash)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "P2WPKH cannot be embedded in P2WSH"); - } - if (keys.size() > 1 || pubKeys.size() > 1) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "More than one key given for one single-key address"); - } - CPubKey pubkey; - if (keys.size()) { - pubkey = DecodeSecret(keys[0].get_str()).GetPubKey(); + // Verify and process input data + bool have_solving_data = import_data.redeemscript || import_data.witnessscript || pubkey_map.size() || privkey_map.size(); + if (have_solving_data) { + // Match up data in import_data with the scriptPubKey in script. + auto error = RecurseImportData(script, import_data, ScriptContext::TOP); + + // Verify whether the watchonly option corresponds to the availability of private keys. + bool spendable = std::all_of(import_data.used_keys.begin(), import_data.used_keys.end(), [&](const std::pair<CKeyID, bool>& used_key){ return privkey_map.count(used_key.first) > 0; }); + if (!watchOnly && !spendable) { + warnings.push_back("Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."); } - if (pubKeys.size()) { - const std::string& strPubKey = pubKeys[0].get_str(); - if (!IsHex(strPubKey)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string"); - } - std::vector<unsigned char> vData(ParseHex(pubKeys[0].get_str())); - CPubKey pubkey_temp(vData.begin(), vData.end()); - if (pubkey.size() && pubkey_temp != pubkey) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key does not match public key for address"); - } - pubkey = pubkey_temp; + if (watchOnly && spendable) { + warnings.push_back("All private keys are provided, outputs will be considered spendable. If this is intentional, do not specify the watchonly flag."); } - if (pubkey.size() > 0) { - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); - } - // Check the key corresponds to the destination given - std::vector<CTxDestination> destinations = GetAllDestinationsForKey(pubkey); - if (std::find(destinations.begin(), destinations.end(), dest) == destinations.end()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Key does not match address destination"); + // Check that all required keys for solvability are provided. + if (error.empty()) { + for (const auto& require_key : import_data.used_keys) { + if (!require_key.second) continue; // Not a required key + if (pubkey_map.count(require_key.first) == 0 && privkey_map.count(require_key.first) == 0) { + error = "some required keys are missing"; + } } + } - // This is necessary to force the wallet to import the pubKey - CScript scriptRawPubKey = GetScriptForRawPubKey(pubkey); - - if (::IsMine(*pwallet, scriptRawPubKey) == ISMINE_SPENDABLE) { - throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); + if (!error.empty()) { + warnings.push_back("Importing as non-solvable: " + error + ". If this is intentional, don't provide any keys, pubkeys, witnessscript, or redeemscript."); + import_data = ImportData(); + pubkey_map.clear(); + privkey_map.clear(); + have_solving_data = false; + } else { + // RecurseImportData() removes any relevant redeemscript/witnessscript from import_data, so we can use that to discover if a superfluous one was provided. + if (import_data.redeemscript) warnings.push_back("Ignoring redeemscript as this is not a P2SH script."); + if (import_data.witnessscript) warnings.push_back("Ignoring witnessscript as this is not a (P2SH-)P2WSH script."); + for (auto it = privkey_map.begin(); it != privkey_map.end(); ) { + auto oldit = it++; + if (import_data.used_keys.count(oldit->first) == 0) { + warnings.push_back("Ignoring irrelevant private key."); + privkey_map.erase(oldit); + } } - - pwallet->MarkDirty(); - - if (!pwallet->AddWatchOnly(scriptRawPubKey, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); + for (auto it = pubkey_map.begin(); it != pubkey_map.end(); ) { + auto oldit = it++; + auto key_data_it = import_data.used_keys.find(oldit->first); + if (key_data_it == import_data.used_keys.end() || !key_data_it->second) { + warnings.push_back("Ignoring public key \"" + HexStr(oldit->first) + "\" as it doesn't appear inside P2PKH or P2WPKH."); + pubkey_map.erase(oldit); + } } } } - // Import the address - if (::IsMine(*pwallet, scriptpubkey_script) == ISMINE_SPENDABLE) { + // Check whether we have any work to do + if (::IsMine(*pwallet, script) & ISMINE_SPENDABLE) { throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); } + // All good, time to import pwallet->MarkDirty(); - - if (!pwallet->AddWatchOnly(scriptpubkey_script, timestamp)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); - } - - if (!watchOnly && !pwallet->HaveCScript(CScriptID(scriptpubkey_script)) && !pwallet->AddCScript(scriptpubkey_script)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding scriptPubKey script to wallet"); - } - - // if not internal add to address book or update label - if (!internal) { - assert(IsValidDestination(scriptpubkey_dest)); - pwallet->SetAddressBook(scriptpubkey_dest, label, "receive"); - } - - // Import private keys. - for (size_t i = 0; i < keys.size(); i++) { - const std::string& strPrivkey = keys[i].get_str(); - - // Checks. - CKey key = DecodeSecret(strPrivkey); - - if (!key.IsValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); + for (const auto& entry : import_data.import_scripts) { + if (!pwallet->HaveCScript(CScriptID(entry)) && !pwallet->AddCScript(entry)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding script to wallet"); } - - CPubKey pubKey = key.GetPubKey(); - assert(key.VerifyPubKey(pubKey)); - - CKeyID vchAddress = pubKey.GetID(); - pwallet->MarkDirty(); - - if (pwallet->HaveKey(vchAddress)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Already have this key"); - } - - pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp; - - if (!pwallet->AddKeyPubKey(key, pubKey)) { + } + for (const auto& entry : privkey_map) { + const CKey& key = entry.second; + CPubKey pubkey = key.GetPubKey(); + const CKeyID& id = entry.first; + assert(key.VerifyPubKey(pubkey)); + pwallet->mapKeyMetadata[id].nCreateTime = timestamp; + // If the private key is not present in the wallet, insert it. + if (!pwallet->HaveKey(id) && !pwallet->AddKeyPubKey(key, pubkey)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } - pwallet->UpdateTimeFirstKey(timestamp); } + for (const auto& entry : pubkey_map) { + const CPubKey& pubkey = entry.second; + const CKeyID& id = entry.first; + CPubKey temp; + if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnly(GetScriptForRawPubKey(pubkey), timestamp)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); + } + } + if (!have_solving_data || !::IsMine(*pwallet, script)) { // Always call AddWatchOnly for non-solvable watch-only, so that watch timestamp gets updated + if (!pwallet->AddWatchOnly(script, timestamp)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); + } + } + if (!internal) { + assert(IsValidDestination(dest)); + pwallet->SetAddressBook(dest, label, "receive"); + } - UniValue result = UniValue(UniValue::VOBJ); result.pushKV("success", UniValue(true)); - return result; } catch (const UniValue& e) { - UniValue result = UniValue(UniValue::VOBJ); result.pushKV("success", UniValue(false)); result.pushKV("error", e); - return result; } catch (...) { - UniValue result = UniValue(UniValue::VOBJ); result.pushKV("success", UniValue(false)); + result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, "Missing required fields")); - return result; } + if (warnings.size()) result.pushKV("warnings", warnings); + return result; } static int64_t GetImportTimestamp(const UniValue& data, int64_t now) @@ -1103,7 +1148,9 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) if (mainRequest.fHelp || mainRequest.params.size() < 1 || mainRequest.params.size() > 2) throw std::runtime_error( RPCHelpMan{"importmulti", - "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), rescanning all addresses in one-shot-only (rescan can be disabled via options). Requires a new wallet backup.\n", + "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), optionally rescanning the blockchain from the earliest creation time of the imported scripts. Requires a new wallet backup.\n" + "If an address/script is imported without all of the private keys required to spend from that address, it will be watchonly. The 'watchonly' option must be set to true in this case or a warning will be returned.\n" + "Conversely, if all the private keys are provided and the address/script is spendable, the watchonly option must be set to false, or a warning will be returned.\n", { {"requests", RPCArg::Type::ARR, /* opt */ false, /* default_val */ "", "Data to be imported", { @@ -1122,18 +1169,18 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) }, {"redeemscript", RPCArg::Type::STR, /* opt */ true, /* default_val */ "omitted", "Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey"}, {"witnessscript", RPCArg::Type::STR, /* opt */ true, /* default_val */ "omitted", "Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey"}, - {"pubkeys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving pubkeys that must occur in the output or redeemscript", + {"pubkeys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving pubkeys to import. They must occur in P2PKH or P2WPKH scripts. They are not required when the private key is also provided (see the \"keys\" argument).", { {"pubKey", RPCArg::Type::STR, /* opt */ false, /* default_val */ "", ""}, } }, - {"keys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript", + {"keys", RPCArg::Type::ARR, /* opt */ true, /* default_val */ "empty array", "Array of strings giving private keys to import. The corresponding public keys must occur in the output or redeemscript.", { {"key", RPCArg::Type::STR, /* opt */ false, /* default_val */ "", ""}, } }, - {"internal", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be treated as not incoming payments aka change"}, - {"watchonly", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be considered watched even when they're not spendable, only allowed if keys are empty"}, + {"internal", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be treated as not incoming payments (also known as change)"}, + {"watchonly", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Stating whether matching outputs should be considered watchonly."}, {"label", RPCArg::Type::STR, /* opt */ true, /* default_val */ "''", "Label to assign to the address, only allowed with internal=false"}, }, }, @@ -1154,7 +1201,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) HelpExampleCli("importmulti", "'[{ \"scriptPubKey\": { \"address\": \"<my address>\" }, \"timestamp\":1455191478 }]' '{ \"rescan\": false}'") + "\nResponse is an array with the same size as the input that has the execution result :\n" - " [{ \"success\": true } , { \"success\": false, \"error\": { \"code\": -1, \"message\": \"Internal Server Error\"} }, ... ]\n"); + " [{\"success\": true}, {\"success\": true, \"warnings\": [\"Ignoring irrelevant private key\"]}, {\"success\": false, \"error\": {\"code\": -1, \"message\": \"Internal Server Error\"}}, ...]\n"); RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ}); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 292bb21cca..5e036eb5df 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1020,15 +1020,12 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request) struct tallyitem { - CAmount nAmount; - int nConf; + CAmount nAmount{0}; + int nConf{std::numeric_limits<int>::max()}; std::vector<uint256> txids; - bool fIsWatchonly; + bool fIsWatchonly{false}; tallyitem() { - nAmount = 0; - nConf = std::numeric_limits<int>::max(); - fIsWatchonly = false; } }; @@ -1397,9 +1394,14 @@ UniValue listtransactions(const JSONRPCRequest& request) "[\n" " {\n" " \"address\":\"address\", (string) The bitcoin address of the transaction.\n" - " \"category\":\"send|receive\", (string) The transaction category.\n" + " \"category\": (string) The transaction category.\n" + " \"send\" Transactions sent.\n" + " \"receive\" Non-coinbase transactions received.\n" + " \"generate\" Coinbase transactions received with more than 100 confirmations.\n" + " \"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" + " \"orphan\" Orphaned coinbase transactions received.\n" " \"amount\": x.xxx, (numeric) The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" - " for the 'receive' category,\n" + " for all other categories\n" " \"label\": \"label\", (string) A comment for the address/transaction, if any\n" " \"vout\": n, (numeric) the vout value\n" " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" @@ -1526,20 +1528,25 @@ static UniValue listsinceblock(const JSONRPCRequest& request) "\nResult:\n" "{\n" " \"transactions\": [\n" - " \"address\":\"address\", (string) The bitcoin address of the transaction. Not present for move transactions (category = move).\n" - " \"category\":\"send|receive\", (string) The transaction category. 'send' has negative amounts, 'receive' has positive amounts.\n" - " \"amount\": x.xxx, (numeric) The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and for the 'move' category for moves \n" - " outbound. It is positive for the 'receive' category, and for the 'move' category for inbound funds.\n" + " \"address\":\"address\", (string) The bitcoin address of the transaction.\n" + " \"category\": (string) The transaction category.\n" + " \"send\" Transactions sent.\n" + " \"receive\" Non-coinbase transactions received.\n" + " \"generate\" Coinbase transactions received with more than 100 confirmations.\n" + " \"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" + " \"orphan\" Orphaned coinbase transactions received.\n" + " \"amount\": x.xxx, (numeric) The amount in " + CURRENCY_UNIT + ". This is negative for the 'send' category, and is positive\n" + " for all other categories\n" " \"vout\" : n, (numeric) the vout value\n" " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the 'send' category of transactions.\n" - " \"confirmations\": n, (numeric) The number of confirmations for the transaction. Available for 'send' and 'receive' category of transactions.\n" + " \"confirmations\": n, (numeric) The number of confirmations for the transaction.\n" " When it's < 0, it means the transaction conflicted that many blocks ago.\n" - " \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction. Available for 'send' and 'receive' category of transactions.\n" - " \"blockindex\": n, (numeric) The index of the transaction in the block that includes it. Available for 'send' and 'receive' category of transactions.\n" + " \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction.\n" + " \"blockindex\": n, (numeric) The index of the transaction in the block that includes it.\n" " \"blocktime\": xxx, (numeric) The block time in seconds since epoch (1 Jan 1970 GMT).\n" - " \"txid\": \"transactionid\", (string) The transaction id. Available for 'send' and 'receive' category of transactions.\n" + " \"txid\": \"transactionid\", (string) The transaction id.\n" " \"time\": xxx, (numeric) The transaction time in seconds since epoch (Jan 1 1970 GMT).\n" - " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (Jan 1 1970 GMT). Available for 'send' and 'receive' category of transactions.\n" + " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (Jan 1 1970 GMT).\n" " \"bip125-replaceable\": \"yes|no|unknown\", (string) Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" " may be unknown for unconfirmed transactions not in the mempool\n" " \"abandoned\": xxx, (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the 'send' category of transactions.\n" @@ -1677,7 +1684,12 @@ static UniValue gettransaction(const JSONRPCRequest& request) " \"details\" : [\n" " {\n" " \"address\" : \"address\", (string) The bitcoin address involved in the transaction\n" - " \"category\" : \"send|receive\", (string) The category, either 'send' or 'receive'\n" + " \"category\" : (string) The transaction category.\n" + " \"send\" Transactions sent.\n" + " \"receive\" Non-coinbase transactions received.\n" + " \"generate\" Coinbase transactions received with more than 100 confirmations.\n" + " \"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" + " \"orphan\" Orphaned coinbase transactions received.\n" " \"amount\" : x.xxx, (numeric) The amount in " + CURRENCY_UNIT + "\n" " \"label\" : \"label\", (string) A comment for the address/transaction, if any\n" " \"vout\" : n, (numeric) the vout value\n" @@ -2625,16 +2637,8 @@ static UniValue unloadwallet(const JSONRPCRequest& request) if (!RemoveWallet(wallet)) { throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } - UnregisterValidationInterface(wallet.get()); - - // The wallet can be in use so it's not possible to explicitly unload here. - // Just notify the unload intent so that all shared pointers are released. - // The wallet will be destroyed once the last shared pointer is released. - wallet->NotifyUnload(); - // There's no point in waiting for the wallet to unload. - // At this point this method should never fail. The unloading could only - // fail due to an unexpected error which would cause a process termination. + UnloadWallet(std::move(wallet)); return NullUniValue; } @@ -3582,7 +3586,6 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " \"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" - " \"solvable\" : true|false, (boolean) If the address is solvable by the wallet\n" " \"iswatchonly\" : true|false, (boolean) If the address is watchonly\n" " \"solvable\" : true|false, (boolean) Whether we know how to spend coins sent to this address, ignoring the possible lack of private keys\n" " \"desc\" : \"desc\", (string, optional) A descriptor for spending coins sent to this address (only when solvable)\n" @@ -3601,7 +3604,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " \"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 getaddressinfo output fields for the embedded address, excluding metadata (\"timestamp\", \"hdkeypath\", \"hdseedid\") and relation to the wallet (\"ismine\", \"iswatchonly\").\n" - " \"iscompressed\" : true|false, (boolean) If the address is compressed\n" + " \"iscompressed\" : true|false, (boolean, optional) If the pubkey is compressed\n" " \"label\" : \"label\" (string) The label associated with the address, \"\" is the default label\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" @@ -3645,7 +3648,6 @@ UniValue getaddressinfo(const JSONRPCRequest& request) ret.pushKV("desc", InferDescriptor(scriptPubKey, *pwallet)->ToString()); } ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY)); - ret.pushKV("solvable", IsSolvable(*pwallet, scriptPubKey)); UniValue detail = DescribeWalletAddress(pwallet, dest); ret.pushKVs(detail); if (pwallet->mapAddressBook.count(dest)) { @@ -4040,8 +4042,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) }, }, }, - {"locktime", RPCArg::Type::NUM, /* opt */ true, /* default_val */ "0", "Raw locktime. Non-0 value also locktime-activates inputs\n" - " Allows this transaction to be replaced by a transaction with higher fees. If provided, it is an error if explicit sequence numbers are incompatible."}, + {"locktime", RPCArg::Type::NUM, /* opt */ true, /* default_val */ "0", "Raw locktime. Non-0 value also locktime-activates inputs"}, {"options", RPCArg::Type::OBJ, /* opt */ true, /* default_val */ "null", "", { {"changeAddress", RPCArg::Type::STR_HEX, /* opt */ true, /* default_val */ "pool address", "The bitcoin address to receive the change"}, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 1ed1926af2..0db22cf6fe 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2012-2018 The Bitcoin Core developers +// Copyright (c) 2012-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -47,7 +47,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) auto locked_chain = chain->lock(); - // Verify ScanForWalletTransactions accomodates a null start block. + // Verify ScanForWalletTransactions accommodates a null start block. { CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy()); AddKey(wallet, coinbaseKey); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bbc66b2839..74deb2dddc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -82,13 +82,52 @@ std::shared_ptr<CWallet> GetWallet(const std::string& name) return nullptr; } +static Mutex g_wallet_release_mutex; +static std::condition_variable g_wallet_release_cv; +static std::set<CWallet*> g_unloading_wallet_set; + // Custom deleter for shared_ptr<CWallet>. static void ReleaseWallet(CWallet* wallet) { + // Unregister and delete the wallet right after BlockUntilSyncedToCurrentChain + // so that it's in sync with the current chainstate. wallet->WalletLogPrintf("Releasing wallet\n"); wallet->BlockUntilSyncedToCurrentChain(); wallet->Flush(); + UnregisterValidationInterface(wallet); delete wallet; + // Wallet is now released, notify UnloadWallet, if any. + { + LOCK(g_wallet_release_mutex); + if (g_unloading_wallet_set.erase(wallet) == 0) { + // UnloadWallet was not called for this wallet, all done. + return; + } + } + g_wallet_release_cv.notify_all(); +} + +void UnloadWallet(std::shared_ptr<CWallet>&& wallet) +{ + // Mark wallet for unloading. + CWallet* pwallet = wallet.get(); + { + LOCK(g_wallet_release_mutex); + auto it = g_unloading_wallet_set.insert(pwallet); + assert(it.second); + } + // The wallet can be in use so it's not possible to explicitly unload here. + // Notify the unload intent so that all remaining shared pointers are + // released. + pwallet->NotifyUnload(); + // Time to ditch our shared_ptr and wait for ReleaseWallet call. + wallet.reset(); + { + WAIT_LOCK(g_wallet_release_mutex, lock); + while (g_unloading_wallet_set.count(pwallet) == 1) { + g_wallet_release_cv.wait(lock); + } + } } const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; @@ -2516,6 +2555,65 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC return true; } +static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock& locked_chain) +{ + if (IsInitialBlockDownload()) { + return false; + } + constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 8 * 60 * 60; // in seconds + if (chainActive.Tip()->GetBlockTime() < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) { + return false; + } + return true; +} + +/** + * Return a height-based locktime for new transactions (uses the height of the + * current chain tip unless we are not synced with the current chain + */ +static uint32_t GetLocktimeForNewTransaction(interfaces::Chain::Lock& locked_chain) +{ + uint32_t locktime; + // Discourage fee sniping. + // + // For a large miner the value of the transactions in the best block and + // the mempool can exceed the cost of deliberately attempting to mine two + // blocks to orphan the current best block. By setting nLockTime such that + // only the next block can include the transaction, we discourage this + // practice as the height restricted and limited blocksize gives miners + // considering fee sniping fewer options for pulling off this attack. + // + // A simple way to think about this is from the wallet's point of view we + // always want the blockchain to move forward. By setting nLockTime this + // way we're basically making the statement that we only want this + // transaction to appear in the next block; we don't want to potentially + // encourage reorgs by allowing transactions to appear at lower heights + // than the next block in forks of the best chain. + // + // Of course, the subsidy is high enough, and transaction volume low + // enough, that fee sniping isn't a problem yet, but by implementing a fix + // now we ensure code won't be written that makes assumptions about + // nLockTime that preclude a fix later. + if (IsCurrentForAntiFeeSniping(locked_chain)) { + locktime = chainActive.Height(); + + // Secondly occasionally randomly pick a nLockTime even further back, so + // that transactions that are delayed after signing for whatever reason, + // e.g. high-latency mix networks and some CoinJoin implementations, have + // better privacy. + if (GetRandInt(10) == 0) + locktime = std::max(0, (int)locktime - GetRandInt(100)); + } else { + // If our chain is lagging behind, we can't discourage fee sniping nor help + // the privacy of high-latency transactions. To avoid leaking a potentially + // unique "nLockTime fingerprint", set nLockTime to a constant. + locktime = 0; + } + assert(locktime <= (unsigned int)chainActive.Height()); + assert(locktime < LOCKTIME_THRESHOLD); + return locktime; +} + OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend) { // If -changetype is specified, always use that change type. @@ -2570,37 +2668,8 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std CMutableTransaction txNew; - // Discourage fee sniping. - // - // For a large miner the value of the transactions in the best block and - // the mempool can exceed the cost of deliberately attempting to mine two - // blocks to orphan the current best block. By setting nLockTime such that - // only the next block can include the transaction, we discourage this - // practice as the height restricted and limited blocksize gives miners - // considering fee sniping fewer options for pulling off this attack. - // - // A simple way to think about this is from the wallet's point of view we - // always want the blockchain to move forward. By setting nLockTime this - // way we're basically making the statement that we only want this - // transaction to appear in the next block; we don't want to potentially - // encourage reorgs by allowing transactions to appear at lower heights - // than the next block in forks of the best chain. - // - // Of course, the subsidy is high enough, and transaction volume low - // enough, that fee sniping isn't a problem yet, but by implementing a fix - // now we ensure code won't be written that makes assumptions about - // nLockTime that preclude a fix later. - txNew.nLockTime = chainActive.Height(); - - // Secondly occasionally randomly pick a nLockTime even further back, so - // that transactions that are delayed after signing for whatever reason, - // e.g. high-latency mix networks and some CoinJoin implementations, have - // better privacy. - if (GetRandInt(10) == 0) - txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100)); + txNew.nLockTime = GetLocktimeForNewTransaction(locked_chain); - assert(txNew.nLockTime <= (unsigned int)chainActive.Height()); - assert(txNew.nLockTime < LOCKTIME_THRESHOLD); FeeCalculation feeCalc; CAmount nFeeNeeded; int nBytes; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 95a2c833f8..6872fbad2d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -54,6 +54,13 @@ void StopWallets(); //! Close all wallets. void UnloadWallets(); +//! Explicitly unload and delete the wallet. +// Blocks the current thread after signaling the unload intent so that all +// wallet clients release the wallet. +// Note that, when blocking is not required, the wallet is implicitly unloaded +// by the shared pointer deleter. +void UnloadWallet(std::shared_ptr<CWallet>&& wallet); + bool AddWallet(const std::shared_ptr<CWallet>& wallet); bool RemoveWallet(const std::shared_ptr<CWallet>& wallet); bool HasWallets(); @@ -770,6 +777,8 @@ public: ~CWallet() { + // Should not have slots connected at this point. + assert(NotifyUnload.empty()); delete encrypted_batch; encrypted_batch = nullptr; } @@ -1178,18 +1187,16 @@ class CReserveKey final : public CReserveScript { protected: CWallet* pwallet; - int64_t nIndex; + int64_t nIndex{-1}; CPubKey vchPubKey; - bool fInternal; + bool fInternal{false}; + public: explicit CReserveKey(CWallet* pwalletIn) { - nIndex = -1; pwallet = pwalletIn; - fInternal = false; } - CReserveKey() = default; CReserveKey(const CReserveKey&) = delete; CReserveKey& operator=(const CReserveKey&) = delete; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 09a33f252c..6e037808e3 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -153,21 +153,17 @@ bool WalletBatch::WriteMinVersion(int nVersion) class CWalletScanState { public: - unsigned int nKeys; - unsigned int nCKeys; - unsigned int nWatchKeys; - unsigned int nKeyMeta; - unsigned int m_unknown_records; - bool fIsEncrypted; - bool fAnyUnordered; - int nFileVersion; + unsigned int nKeys{0}; + unsigned int nCKeys{0}; + unsigned int nWatchKeys{0}; + unsigned int nKeyMeta{0}; + unsigned int m_unknown_records{0}; + bool fIsEncrypted{false}; + bool fAnyUnordered{false}; + int nFileVersion{0}; std::vector<uint256> vWalletUpgrade; CWalletScanState() { - nKeys = nCKeys = nWatchKeys = nKeyMeta = m_unknown_records = 0; - fIsEncrypted = false; - fAnyUnordered = false; - nFileVersion = 0; } }; |