diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/crypter.cpp | 139 | ||||
-rw-r--r-- | src/wallet/crypter.h | 44 | ||||
-rw-r--r-- | src/wallet/db.cpp | 94 | ||||
-rw-r--r-- | src/wallet/db.h | 2 | ||||
-rw-r--r-- | src/wallet/feebumper.cpp | 18 | ||||
-rw-r--r-- | src/wallet/init.cpp | 18 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 17 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 39 | ||||
-rw-r--r-- | src/wallet/test/accounting_tests.cpp | 4 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.cpp | 11 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 5 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 27 | ||||
-rw-r--r-- | src/wallet/wallet.h | 4 | ||||
-rw-r--r-- | src/wallet/walletdb.cpp | 2 |
14 files changed, 235 insertions, 189 deletions
diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 8db3bfd69c..5b31a40fc7 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -152,6 +152,15 @@ bool CCryptoKeyStore::SetCrypted() return true; } +bool CCryptoKeyStore::IsLocked() const +{ + if (!IsCrypted()) { + return false; + } + LOCK(cs_KeyStore); + return vMasterKey.empty(); +} + bool CCryptoKeyStore::Lock() { if (!SetCrypted()) @@ -206,21 +215,23 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) bool CCryptoKeyStore::AddKeyPubKey(const CKey& key, const CPubKey &pubkey) { - { - LOCK(cs_KeyStore); - if (!IsCrypted()) - return CBasicKeyStore::AddKeyPubKey(key, pubkey); + LOCK(cs_KeyStore); + if (!IsCrypted()) { + return CBasicKeyStore::AddKeyPubKey(key, pubkey); + } - if (IsLocked()) - return false; + if (IsLocked()) { + return false; + } - std::vector<unsigned char> vchCryptedSecret; - CKeyingMaterial vchSecret(key.begin(), key.end()); - if (!EncryptSecret(vMasterKey, vchSecret, pubkey.GetHash(), vchCryptedSecret)) - return false; + std::vector<unsigned char> vchCryptedSecret; + CKeyingMaterial vchSecret(key.begin(), key.end()); + if (!EncryptSecret(vMasterKey, vchSecret, pubkey.GetHash(), vchCryptedSecret)) { + return false; + } - if (!AddCryptedKey(pubkey, vchCryptedSecret)) - return false; + if (!AddCryptedKey(pubkey, vchCryptedSecret)) { + return false; } return true; } @@ -228,72 +239,88 @@ bool CCryptoKeyStore::AddKeyPubKey(const CKey& key, const CPubKey &pubkey) bool CCryptoKeyStore::AddCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret) { - { - LOCK(cs_KeyStore); - if (!SetCrypted()) - return false; - - mapCryptedKeys[vchPubKey.GetID()] = make_pair(vchPubKey, vchCryptedSecret); + LOCK(cs_KeyStore); + if (!SetCrypted()) { + return false; } + + mapCryptedKeys[vchPubKey.GetID()] = make_pair(vchPubKey, vchCryptedSecret); return true; } +bool CCryptoKeyStore::HaveKey(const CKeyID &address) const +{ + LOCK(cs_KeyStore); + if (!IsCrypted()) { + return CBasicKeyStore::HaveKey(address); + } + return mapCryptedKeys.count(address) > 0; +} + bool CCryptoKeyStore::GetKey(const CKeyID &address, CKey& keyOut) const { - { - LOCK(cs_KeyStore); - if (!IsCrypted()) - return CBasicKeyStore::GetKey(address, keyOut); + LOCK(cs_KeyStore); + if (!IsCrypted()) { + return CBasicKeyStore::GetKey(address, keyOut); + } - CryptedKeyMap::const_iterator mi = mapCryptedKeys.find(address); - if (mi != mapCryptedKeys.end()) - { - const CPubKey &vchPubKey = (*mi).second.first; - const std::vector<unsigned char> &vchCryptedSecret = (*mi).second.second; - return DecryptKey(vMasterKey, vchCryptedSecret, vchPubKey, keyOut); - } + CryptedKeyMap::const_iterator mi = mapCryptedKeys.find(address); + if (mi != mapCryptedKeys.end()) + { + const CPubKey &vchPubKey = (*mi).second.first; + const std::vector<unsigned char> &vchCryptedSecret = (*mi).second.second; + return DecryptKey(vMasterKey, vchCryptedSecret, vchPubKey, keyOut); } return false; } bool CCryptoKeyStore::GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const { + LOCK(cs_KeyStore); + if (!IsCrypted()) + return CBasicKeyStore::GetPubKey(address, vchPubKeyOut); + + CryptedKeyMap::const_iterator mi = mapCryptedKeys.find(address); + if (mi != mapCryptedKeys.end()) { - LOCK(cs_KeyStore); - if (!IsCrypted()) - return CBasicKeyStore::GetPubKey(address, vchPubKeyOut); + vchPubKeyOut = (*mi).second.first; + return true; + } + // Check for watch-only pubkeys + return CBasicKeyStore::GetPubKey(address, vchPubKeyOut); +} - CryptedKeyMap::const_iterator mi = mapCryptedKeys.find(address); - if (mi != mapCryptedKeys.end()) - { - vchPubKeyOut = (*mi).second.first; - return true; - } - // Check for watch-only pubkeys - return CBasicKeyStore::GetPubKey(address, vchPubKeyOut); +std::set<CKeyID> CCryptoKeyStore::GetKeys() const +{ + LOCK(cs_KeyStore); + if (!IsCrypted()) { + return CBasicKeyStore::GetKeys(); } + std::set<CKeyID> set_address; + for (const auto& mi : mapCryptedKeys) { + set_address.insert(mi.first); + } + return set_address; } bool CCryptoKeyStore::EncryptKeys(CKeyingMaterial& vMasterKeyIn) { + LOCK(cs_KeyStore); + if (!mapCryptedKeys.empty() || IsCrypted()) + return false; + + fUseCrypto = true; + for (KeyMap::value_type& mKey : mapKeys) { - LOCK(cs_KeyStore); - if (!mapCryptedKeys.empty() || IsCrypted()) + const CKey &key = mKey.second; + CPubKey vchPubKey = key.GetPubKey(); + CKeyingMaterial vchSecret(key.begin(), key.end()); + std::vector<unsigned char> vchCryptedSecret; + if (!EncryptSecret(vMasterKeyIn, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) + return false; + if (!AddCryptedKey(vchPubKey, vchCryptedSecret)) return false; - - fUseCrypto = true; - for (KeyMap::value_type& mKey : mapKeys) - { - const CKey &key = mKey.second; - CPubKey vchPubKey = key.GetPubKey(); - CKeyingMaterial vchSecret(key.begin(), key.end()); - std::vector<unsigned char> vchCryptedSecret; - if (!EncryptSecret(vMasterKeyIn, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) - return false; - if (!AddCryptedKey(vchPubKey, vchCryptedSecret)) - return false; - } - mapKeys.clear(); } + mapKeys.clear(); return true; } diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 1416ae7d02..67c8481196 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -139,52 +139,16 @@ public: { } - bool IsCrypted() const - { - return fUseCrypto; - } - - bool IsLocked() const - { - if (!IsCrypted()) - return false; - bool result; - { - LOCK(cs_KeyStore); - result = vMasterKey.empty(); - } - return result; - } - + bool IsCrypted() const { return fUseCrypto; } + bool IsLocked() const; bool Lock(); virtual bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret); bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey) override; - bool HaveKey(const CKeyID &address) const override - { - { - LOCK(cs_KeyStore); - if (!IsCrypted()) { - return CBasicKeyStore::HaveKey(address); - } - return mapCryptedKeys.count(address) > 0; - } - return false; - } + bool HaveKey(const CKeyID &address) const override; bool GetKey(const CKeyID &address, CKey& keyOut) const override; bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const override; - std::set<CKeyID> GetKeys() const override - { - LOCK(cs_KeyStore); - if (!IsCrypted()) { - return CBasicKeyStore::GetKeys(); - } - std::set<CKeyID> set_address; - for (const auto& mi : mapCryptedKeys) { - set_address.insert(mi.first); - } - return set_address; - } + std::set<CKeyID> GetKeys() const override; /** * Wallet status (encrypted, locked) changed. diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index d66ba48421..ca8e680ff7 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -20,6 +20,40 @@ #include <boost/thread.hpp> +namespace { +//! Make sure database has a unique fileid within the environment. If it +//! doesn't, throw an error. BDB caches do not work properly when more than one +//! open database has the same fileid (values written to one database may show +//! up in reads to other databases). +//! +//! BerkeleyDB generates unique fileids by default +//! (https://docs.oracle.com/cd/E17275_01/html/programmer_reference/program_copy.html), +//! so bitcoin should never create different databases with the same fileid, but +//! this error can be triggered if users manually copy database files. +void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) +{ + if (env.IsMock()) return; + + u_int8_t fileid[DB_FILE_ID_LEN]; + int ret = db.get_mpf()->get_fileid(fileid); + if (ret != 0) { + throw std::runtime_error(strprintf("CDB: Can't open database %s (get_fileid failed with %d)", filename, ret)); + } + + for (const auto& item : env.mapDb) { + u_int8_t item_fileid[DB_FILE_ID_LEN]; + if (item.second && item.second->get_mpf()->get_fileid(item_fileid) == 0 && + memcmp(fileid, item_fileid, sizeof(fileid)) == 0) { + const char* item_filename = nullptr; + item.second->get_dbname(&item_filename, nullptr); + throw std::runtime_error(strprintf("CDB: Can't open database %s (duplicates fileid %s from %s)", filename, + HexStr(std::begin(item_fileid), std::end(item_fileid)), + item_filename ? item_filename : "(unknown database)")); + } + } +} +} // namespace + // // CDB // @@ -41,13 +75,12 @@ void CDBEnv::EnvShutdown() void CDBEnv::Reset() { - delete dbenv; - dbenv = new DbEnv(DB_CXX_NO_EXCEPTIONS); + dbenv.reset(new DbEnv(DB_CXX_NO_EXCEPTIONS)); fDbEnvInit = false; fMockDb = false; } -CDBEnv::CDBEnv() : dbenv(nullptr) +CDBEnv::CDBEnv() { Reset(); } @@ -55,8 +88,6 @@ CDBEnv::CDBEnv() : dbenv(nullptr) CDBEnv::~CDBEnv() { EnvShutdown(); - delete dbenv; - dbenv = nullptr; } void CDBEnv::Close() @@ -148,7 +179,7 @@ CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, recoverFunc_type LOCK(cs_db); assert(mapFileUseCount.count(strFile) == 0); - Db db(dbenv, 0); + Db db(dbenv.get(), 0); int result = db.verify(strFile.c_str(), nullptr, nullptr, 0); if (result == 0) return VERIFY_OK; @@ -191,7 +222,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - std::unique_ptr<Db> pdbCopy(new Db(bitdb.dbenv, 0)); + std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(bitdb.dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer filename.c_str(), // Filename "main", // Logical db name @@ -300,7 +331,7 @@ bool CDBEnv::Salvage(const std::string& strFile, bool fAggressive, std::vector<C std::stringstream strDump; - Db db(dbenv, 0); + Db db(dbenv.get(), 0); int result = db.verify(strFile.c_str(), nullptr, &strDump, flags); if (result == DB_VERIFY_BAD) { LogPrintf("CDBEnv::Salvage: Database salvage found errors, all data may not be recoverable.\n"); @@ -379,35 +410,34 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (!env->Open(GetDataDir())) throw std::runtime_error("CDB: Failed to open database environment."); - strFile = strFilename; - ++env->mapFileUseCount[strFile]; - pdb = env->mapDb[strFile]; + pdb = env->mapDb[strFilename]; if (pdb == nullptr) { int ret; - pdb = new Db(env->dbenv, 0); + std::unique_ptr<Db> pdb_temp = MakeUnique<Db>(env->dbenv.get(), 0); bool fMockDb = env->IsMock(); if (fMockDb) { - DbMpoolFile* mpf = pdb->get_mpf(); + DbMpoolFile* mpf = pdb_temp->get_mpf(); ret = mpf->set_flags(DB_MPOOL_NOFILE, 1); - if (ret != 0) - throw std::runtime_error(strprintf("CDB: Failed to configure for no temp file backing for database %s", strFile)); + if (ret != 0) { + throw std::runtime_error(strprintf("CDB: Failed to configure for no temp file backing for database %s", strFilename)); + } } - ret = pdb->open(nullptr, // Txn pointer - fMockDb ? nullptr : strFile.c_str(), // Filename - fMockDb ? strFile.c_str() : "main", // Logical db name - DB_BTREE, // Database type - nFlags, // Flags + ret = pdb_temp->open(nullptr, // Txn pointer + fMockDb ? nullptr : strFilename.c_str(), // Filename + fMockDb ? strFilename.c_str() : "main", // Logical db name + DB_BTREE, // Database type + nFlags, // Flags 0); if (ret != 0) { - delete pdb; - pdb = nullptr; - --env->mapFileUseCount[strFile]; - strFile = ""; throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename)); } + CheckUniqueFileid(*env, strFilename, *pdb_temp); + + pdb = pdb_temp.release(); + env->mapDb[strFilename] = pdb; if (fCreate && !Exists(std::string("version"))) { bool fTmp = fReadOnly; @@ -415,9 +445,9 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb WriteVersion(CLIENT_VERSION); fReadOnly = fTmp; } - - env->mapDb[strFile] = pdb; } + ++env->mapFileUseCount[strFilename]; + strFile = strFilename; } } @@ -492,7 +522,7 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) std::string strFileRes = strFile + ".rewrite"; { // surround usage of db with extra {} CDB db(dbw, "r"); - Db* pdbCopy = new Db(env->dbenv, 0); + std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer strFileRes.c_str(), // Filename @@ -541,13 +571,12 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) } else { pdbCopy->close(0); } - delete pdbCopy; } if (fSuccess) { - Db dbA(env->dbenv, 0); + Db dbA(env->dbenv.get(), 0); if (dbA.remove(strFile.c_str(), nullptr, 0)) fSuccess = false; - Db dbB(env->dbenv, 0); + Db dbB(env->dbenv.get(), 0); if (dbB.rename(strFileRes.c_str(), nullptr, strFile.c_str(), 0)) fSuccess = false; } @@ -672,6 +701,11 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) pathDest /= strFile; try { + if (fs::equivalent(pathSrc, pathDest)) { + LogPrintf("cannot backup to wallet source file %s\n", pathDest.string()); + return false; + } + fs::copy_file(pathSrc, pathDest, fs::copy_option::overwrite_if_exists); LogPrintf("copied %s to %s\n", strFile, pathDest.string()); return true; diff --git a/src/wallet/db.h b/src/wallet/db.h index 14283ac8f8..e6d5a9f293 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -36,7 +36,7 @@ private: public: mutable CCriticalSection cs_db; - DbEnv *dbenv; + std::unique_ptr<DbEnv> dbenv; std::map<std::string, int> mapFileUseCount; std::map<std::string, Db*> mapDb; diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 6abd060714..8b7c50b4e9 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -89,7 +89,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin return; } - if (!SignalsOptInRBF(wtx)) { + if (!SignalsOptInRBF(*wtx.tx)) { vErrors.push_back("Transaction is not BIP 125 replaceable"); currentResult = BumpFeeResult::WALLET_ERROR; return; @@ -103,7 +103,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin // check that original tx consists entirely of our inputs // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) - if (!pWallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) { + if (!pWallet->IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) { vErrors.push_back("Transaction contains inputs that don't belong to this wallet"); currentResult = BumpFeeResult::WALLET_ERROR; return; @@ -196,7 +196,13 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin // moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment. CFeeRate minMempoolFeeRate = mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) { - vErrors.push_back(strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK()))); + vErrors.push_back(strprintf( + "New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- " + "the totalFee value should be at least %s or the settxfee value should be at least %s to add transaction", + FormatMoney(nNewFeeRate.GetFeePerK()), + FormatMoney(minMempoolFeeRate.GetFeePerK()), + FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), + FormatMoney(minMempoolFeeRate.GetFeePerK()))); currentResult = BumpFeeResult::WALLET_ERROR; return; } @@ -267,7 +273,7 @@ bool CFeeBumper::commit(CWallet *pWallet) CValidationState state; if (!pWallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { // NOTE: CommitTransaction never returns false, so this should never happen. - vErrors.push_back(strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason())); + vErrors.push_back(strprintf("The transaction was rejected: %s", state.GetRejectReason())); return false; } @@ -275,7 +281,7 @@ bool CFeeBumper::commit(CWallet *pWallet) if (state.IsInvalid()) { // This can happen if the mempool rejected the transaction. Report // what happened in the "errors" response. - vErrors.push_back(strprintf("Error: The transaction was rejected: %s", FormatStateMessage(state))); + vErrors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); } // mark the original tx as bumped @@ -284,7 +290,7 @@ bool CFeeBumper::commit(CWallet *pWallet) // along with an exception. It would be good to return information about // wtxBumped to the caller even if marking the original transaction // replaced does not succeed for some reason. - vErrors.push_back("Error: Created new bumpfee transaction but could not mark the original transaction as replaced."); + vErrors.push_back("Created new bumpfee transaction but could not mark the original transaction as replaced"); } return true; } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index c984df1df8..6b5d4cc668 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -53,11 +53,16 @@ std::string GetWalletHelpString(bool showDebug) bool WalletParameterInteraction() { - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); - const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; + if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { + for (const std::string& wallet : gArgs.GetArgs("-wallet")) { + LogPrintf("%s: parameter interaction: -disablewallet -> ignoring -wallet=%s\n", __func__, wallet); + } - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) return true; + } + + gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); @@ -173,15 +178,18 @@ bool WalletParameterInteraction() void RegisterWalletRPC(CRPCTable &t) { - if (gArgs.GetBoolArg("-disablewallet", false)) return; + if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { + return; + } RegisterWalletRPCCommands(t); } bool VerifyWallets() { - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) + if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { return true; + } uiInterface.InitMessage(_("Verifying wallet(s)...")); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d6ea2a9db7..30246a534b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -81,7 +81,7 @@ UniValue importprivkey(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) throw std::runtime_error( "importprivkey \"privkey\" ( \"label\" ) ( rescan )\n" - "\nAdds a private key (as returned by dumpprivkey) to your wallet.\n" + "\nAdds a private key (as returned by dumpprivkey) to your wallet. Requires a new wallet backup.\n" "\nArguments:\n" "1. \"privkey\" (string, required) The private key (see dumpprivkey)\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" @@ -226,7 +226,7 @@ UniValue importaddress(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) throw std::runtime_error( "importaddress \"address\" ( \"label\" rescan p2sh )\n" - "\nAdds a script (in hex) or address that can be watched as if it were in your wallet but cannot be used to spend.\n" + "\nAdds a script (in hex) or address that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nArguments:\n" "1. \"script\" (string, required) The hex-encoded script (or address)\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" @@ -340,7 +340,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (pwallet->IsMine(wtx)) { + if (pwallet->IsMine(*wtx.tx)) { pwallet->AddToWallet(wtx, false); return NullUniValue; } @@ -396,7 +396,7 @@ UniValue importpubkey(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) throw std::runtime_error( "importpubkey \"pubkey\" ( \"label\" rescan )\n" - "\nAdds a public key (in hex) that can be watched as if it were in your wallet but cannot be used to spend.\n" + "\nAdds a public key (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nArguments:\n" "1. \"pubkey\" (string, required) The hex-encoded public key\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" @@ -456,7 +456,7 @@ UniValue importwallet(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw std::runtime_error( "importwallet \"filename\"\n" - "\nImports keys from a wallet dump file (see dumpwallet).\n" + "\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n" "\nArguments:\n" "1. \"filename\" (string, required) The wallet file\n" "\nExamples:\n" @@ -601,6 +601,9 @@ UniValue dumpwallet(const JSONRPCRequest& request) throw std::runtime_error( "dumpwallet \"filename\"\n" "\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n" + "Imported scripts are not currently included in wallet dumps, these must be backed up separately.\n" + "Note that if your wallet contains keys which are not derived from your HD seed (e.g. imported keys), these are not covered by\n" + "only backing up the seed itself, and must be backed up too (e.g. ensure you back up the whole dumpfile).\n" "\nArguments:\n" "1. \"filename\" (string, required) The filename with path (either absolute or relative to bitcoind)\n" "\nResult:\n" @@ -961,7 +964,7 @@ UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int6 pwallet->SetAddressBook(vchAddress, label, "receive"); if (pwallet->HaveKey(vchAddress)) { - return false; + throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); } pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp; @@ -1039,7 +1042,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) if (mainRequest.fHelp || mainRequest.params.size() < 1 || mainRequest.params.size() > 2) throw std::runtime_error( "importmulti \"requests\" ( \"options\" )\n\n" - "Import addresses/scripts (with private or public keys, redeem script (P2SH)), rescanning all addresses in one-shot-only (rescan can be disabled via options).\n\n" + "Import 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\n" "Arguments:\n" "1. requests (array, required) Data to be imported\n" " [ (array of json objects)\n" diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6d01f79d52..cba0538e7e 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -108,7 +108,7 @@ void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry) std::string rbfStatus = "no"; if (confirms <= 0) { LOCK(mempool.cs); - RBFTransactionState rbfState = IsRBFOptIn(wtx, mempool); + RBFTransactionState rbfState = IsRBFOptIn(*wtx.tx, mempool); if (rbfState == RBF_TRANSACTIONSTATE_UNKNOWN) rbfStatus = "unknown"; else if (rbfState == RBF_TRANSACTIONSTATE_REPLACEABLE_BIP125) @@ -669,7 +669,7 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) } CScript scriptPubKey = GetScriptForDestination(dest); if (!IsMine(*pwallet, scriptPubKey)) { - return ValueFromAmount(0); + throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet"); } // Minimum confirmations @@ -1150,7 +1150,7 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 2 || request.params.size() > 3) { std::string msg = "addmultisigaddress nrequired [\"key\",...] ( \"account\" )\n" - "\nAdd a nrequired-to-sign multisignature address to the wallet.\n" + "\nAdd a nrequired-to-sign multisignature address to the wallet. Requires a new wallet backup.\n" "Each key is a Bitcoin address or hex-encoded public key.\n" "If 'account' is specified (DEPRECATED), assign address to that account.\n" @@ -1268,7 +1268,7 @@ UniValue addwitnessaddress(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) { std::string msg = "addwitnessaddress \"address\" ( p2sh )\n" - "\nAdd a witness address for a script (with pubkey or redeemscript known).\n" + "\nAdd a witness address for a script (with pubkey or redeemscript known). Requires a new wallet backup.\n" "It returns the witness script.\n" "\nArguments:\n" @@ -1958,19 +1958,20 @@ UniValue listsinceblock(const JSONRPCRequest& request) int target_confirms = 1; isminefilter filter = ISMINE_SPENDABLE; - if (!request.params[0].isNull()) { + if (!request.params[0].isNull() && !request.params[0].get_str().empty()) { uint256 blockId; blockId.SetHex(request.params[0].get_str()); BlockMap::iterator it = mapBlockIndex.find(blockId); - if (it != mapBlockIndex.end()) { - paltindex = pindex = it->second; - if (chainActive[pindex->nHeight] != pindex) { - // the block being asked for is a part of a deactivated chain; - // we don't want to depend on its perceived height in the block - // chain, we want to instead use the last common ancestor - pindex = chainActive.FindFork(pindex); - } + if (it == mapBlockIndex.end()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } + paltindex = pindex = it->second; + if (chainActive[pindex->nHeight] != pindex) { + // the block being asked for is a part of a deactivated chain; + // we don't want to depend on its perceived height in the block + // chain, we want to instead use the last common ancestor + pindex = chainActive.FindFork(pindex); } } @@ -2120,7 +2121,7 @@ UniValue gettransaction(const JSONRPCRequest& request) ListTransactions(pwallet, wtx, "*", 0, false, details, filter); entry.push_back(Pair("details", details)); - std::string strHex = EncodeHexTx(static_cast<CTransaction>(wtx), RPCSerializationFlags()); + std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags()); entry.push_back(Pair("hex", strHex)); return entry; @@ -2258,7 +2259,7 @@ UniValue walletpassphrase(const JSONRPCRequest& request) return NullUniValue; } - if (pwallet->IsCrypted() && (request.fHelp || request.params.size() != 2)) { + if (request.fHelp || request.params.size() != 2) { throw std::runtime_error( "walletpassphrase \"passphrase\" timeout\n" "\nStores the wallet decryption key in memory for 'timeout' seconds.\n" @@ -2322,7 +2323,7 @@ UniValue walletpassphrasechange(const JSONRPCRequest& request) return NullUniValue; } - if (pwallet->IsCrypted() && (request.fHelp || request.params.size() != 2)) { + if (request.fHelp || request.params.size() != 2) { throw std::runtime_error( "walletpassphrasechange \"oldpassphrase\" \"newpassphrase\"\n" "\nChanges the wallet passphrase from 'oldpassphrase' to 'newpassphrase'.\n" @@ -2373,7 +2374,7 @@ UniValue walletlock(const JSONRPCRequest& request) return NullUniValue; } - if (pwallet->IsCrypted() && (request.fHelp || request.params.size() != 0)) { + if (request.fHelp || request.params.size() != 0) { throw std::runtime_error( "walletlock\n" "\nRemoves the wallet encryption key from memory, locking the wallet.\n" @@ -2413,7 +2414,7 @@ UniValue encryptwallet(const JSONRPCRequest& request) return NullUniValue; } - if (!pwallet->IsCrypted() && (request.fHelp || request.params.size() != 1)) { + if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( "encryptwallet \"passphrase\"\n" "\nEncrypts the wallet with 'passphrase'. This is for first time encryption.\n" @@ -3332,7 +3333,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) "}\n" "\nExamples:\n" + HelpExampleCli("rescanblockchain", "100000 120000") - + HelpExampleRpc("rescanblockchain", "100000 120000") + + HelpExampleRpc("rescanblockchain", "100000, 120000") ); } diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp index 330878ceb5..64244dceea 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "y"; { - CMutableTransaction tx(wtx); + CMutableTransaction tx(*wtx.tx); --tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } @@ -93,7 +93,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "x"; { - CMutableTransaction tx(wtx); + CMutableTransaction tx(*wtx.tx); --tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index e2f48c45ab..34f210fcd7 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,7 +8,7 @@ #include "wallet/db.h" #include "wallet/wallet.h" -CWallet *pwalletMain; +std::unique_ptr<CWallet> pwalletMain; WalletTestingSetup::WalletTestingSetup(const std::string& chainName): TestingSetup(chainName) @@ -17,18 +17,17 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): bool fFirstRun; std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - pwalletMain = new CWallet(std::move(dbw)); + pwalletMain = MakeUnique<CWallet>(std::move(dbw)); pwalletMain->LoadWallet(fFirstRun); - RegisterValidationInterface(pwalletMain); + RegisterValidationInterface(pwalletMain.get()); RegisterWalletRPCCommands(tableRPC); } WalletTestingSetup::~WalletTestingSetup() { - UnregisterValidationInterface(pwalletMain); - delete pwalletMain; - pwalletMain = nullptr; + UnregisterValidationInterface(pwalletMain.get()); + pwalletMain.reset(); bitdb.Flush(true); bitdb.Reset(); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 2b12168c65..503dedb5b3 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -489,6 +489,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) vpwallets[0] = &wallet; ::importwallet(request); + LOCK(wallet.cs_wallet); BOOST_CHECK_EQUAL(wallet.mapWallet.size(), 3); BOOST_CHECK_EQUAL(coinbaseTxns.size(), 103); for (size_t i = 0; i < coinbaseTxns.size(); ++i) { @@ -534,6 +535,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 SetMockTime(mockTime); CBlockIndex* block = nullptr; if (blockTime > 0) { + LOCK(cs_main); auto inserted = mapBlockIndex.emplace(GetRandHash(), new CBlockIndex); assert(inserted.second); const uint256& hash = inserted.first->first; @@ -547,6 +549,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 wtx.SetMerkleBranch(block, 0); } wallet.AddToWallet(wtx); + LOCK(wallet.cs_wallet); return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; } @@ -583,6 +586,7 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart) BOOST_AUTO_TEST_CASE(LoadReceiveRequests) { CTxDestination dest = CKeyID(); + LOCK(pwalletMain->cs_wallet); pwalletMain->AddDestData(dest, "misc", "val_misc"); pwalletMain->AddDestData(dest, "rr0", "val_rr0"); pwalletMain->AddDestData(dest, "rr1", "val_rr1"); @@ -625,6 +629,7 @@ public: BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); CValidationState state; BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(wtx.GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index eee66ec582..8eb84d397f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -533,6 +533,9 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran copyFrom = &mapWallet[hash]; } } + + assert(copyFrom); + // Now copy data from copyFrom to rest: for (TxSpends::iterator it = range.first; it != range.second; ++it) { @@ -1766,7 +1769,7 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const debit += nDebitCached; else { - nDebitCached = pwallet->GetDebit(*this, ISMINE_SPENDABLE); + nDebitCached = pwallet->GetDebit(*tx, ISMINE_SPENDABLE); fDebitCached = true; debit += nDebitCached; } @@ -1777,7 +1780,7 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const debit += nWatchDebitCached; else { - nWatchDebitCached = pwallet->GetDebit(*this, ISMINE_WATCH_ONLY); + nWatchDebitCached = pwallet->GetDebit(*tx, ISMINE_WATCH_ONLY); fWatchDebitCached = true; debit += nWatchDebitCached; } @@ -1799,7 +1802,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const credit += nCreditCached; else { - nCreditCached = pwallet->GetCredit(*this, ISMINE_SPENDABLE); + nCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE); fCreditCached = true; credit += nCreditCached; } @@ -1810,7 +1813,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const credit += nWatchCreditCached; else { - nWatchCreditCached = pwallet->GetCredit(*this, ISMINE_WATCH_ONLY); + nWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY); fWatchCreditCached = true; credit += nWatchCreditCached; } @@ -1824,7 +1827,7 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const { if (fUseCache && fImmatureCreditCached) return nImmatureCreditCached; - nImmatureCreditCached = pwallet->GetCredit(*this, ISMINE_SPENDABLE); + nImmatureCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE); fImmatureCreditCached = true; return nImmatureCreditCached; } @@ -1868,7 +1871,7 @@ CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool& fUseCache) const { if (fUseCache && fImmatureWatchCreditCached) return nImmatureWatchCreditCached; - nImmatureWatchCreditCached = pwallet->GetCredit(*this, ISMINE_WATCH_ONLY); + nImmatureWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY); fImmatureWatchCreditCached = true; return nImmatureWatchCreditCached; } @@ -1909,7 +1912,7 @@ CAmount CWalletTx::GetChange() const { if (fChangeCached) return nChangeCached; - nChangeCached = pwallet->GetChange(*this); + nChangeCached = pwallet->GetChange(*tx); fChangeCached = true; return nChangeCached; } @@ -1922,7 +1925,7 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted() const { // Quick answer in most cases - if (!CheckFinalTx(*this)) + if (!CheckFinalTx(*tx)) return false; int nDepth = GetDepthInMainChain(); if (nDepth >= 1) @@ -2180,7 +2183,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const const uint256& wtxid = it->first; const CWalletTx* pcoin = &(*it).second; - if (!CheckFinalTx(*pcoin)) + if (!CheckFinalTx(*pcoin->tx)) continue; if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0) @@ -2962,7 +2965,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT wtxNew.SetTx(MakeTransactionRef(std::move(txNew))); // Limit size - if (GetTransactionWeight(wtxNew) >= MAX_STANDARD_TX_WEIGHT) + if (GetTransactionWeight(*wtxNew.tx) >= MAX_STANDARD_TX_WEIGHT) { strFailReason = _("Transaction too large"); return false; @@ -3854,7 +3857,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) uiInterface.InitMessage(_("Zapping all transactions from wallet...")); std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile)); - std::unique_ptr<CWallet> tempWallet(new CWallet(std::move(dbw))); + std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(std::move(dbw)); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DB_LOAD_OK) { InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); @@ -3932,7 +3935,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) // Top up the keypool if (!walletInstance->TopUpKeyPool()) { InitError(_("Unable to generate initial keys") += "\n"); - return NULL; + return nullptr; } walletInstance->SetBestChain(chainActive.GetLocator()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 73da686364..c372f3ba19 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -214,10 +214,6 @@ public: Init(); } - /** Helper conversion operator to allow passing CMerkleTx where CTransaction is expected. - * TODO: adapt callers and remove this operator. */ - operator const CTransaction&() const { return *tx; } - void Init() { hashBlock = uint256(); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index b7f873c1e4..180706b1ed 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -268,7 +268,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, CWalletTx wtx; ssValue >> wtx; CValidationState state; - if (!(CheckTransaction(wtx, state) && (wtx.GetHash() == hash) && state.IsValid())) + if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid())) return false; // Undo serialize changes in 31600 |