diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/crypter.h | 2 | ||||
-rw-r--r-- | src/wallet/db.cpp | 12 | ||||
-rw-r--r-- | src/wallet/db.h | 2 | ||||
-rw-r--r-- | src/wallet/feebumper.cpp | 9 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 106 | ||||
-rw-r--r-- | src/wallet/test/crypto_tests.cpp | 133 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.h | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 104 | ||||
-rw-r--r-- | src/wallet/wallet.h | 18 | ||||
-rw-r--r-- | src/wallet/walletdb.cpp | 19 | ||||
-rw-r--r-- | src/wallet/walletdb.h | 6 |
11 files changed, 153 insertions, 260 deletions
diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 1dc44e424f..f1e8a25650 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -113,7 +113,6 @@ public: class CCryptoKeyStore : public CBasicKeyStore { private: - CryptedKeyMap mapCryptedKeys; CKeyingMaterial vMasterKey; @@ -131,6 +130,7 @@ protected: bool EncryptKeys(CKeyingMaterial& vMasterKeyIn); bool Unlock(const CKeyingMaterial& vMasterKeyIn); + CryptedKeyMap mapCryptedKeys; public: CCryptoKeyStore() : fUseCrypto(false), fDecryptionThoroughlyChecked(false) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index b12d46e40a..d2fe4866fa 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -101,8 +101,10 @@ bool CDBEnv::Open(const fs::path& pathIn) DB_RECOVER | nEnvFlags, S_IRUSR | S_IWUSR); - if (ret != 0) + if (ret != 0) { + dbenv->close(0); return error("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + } fDbEnvInit = true; fMockDb = false; @@ -196,9 +198,9 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco DB_BTREE, // Database type DB_CREATE, // Flags 0); - if (ret > 0) - { + if (ret > 0) { LogPrintf("Cannot create database file %s\n", filename); + pdbCopy->close(0); return false; } @@ -536,8 +538,10 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) env->CloseDb(strFile); if (pdbCopy->close(0)) fSuccess = false; - delete pdbCopy; + } else { + pdbCopy->close(0); } + delete pdbCopy; } if (fSuccess) { Db dbA(env->dbenv, 0); diff --git a/src/wallet/db.h b/src/wallet/db.h index 3614e34fbb..6f3cfe9557 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -45,7 +45,7 @@ public: void Reset(); void MakeMock(); - bool IsMock() { return fMockDb; } + bool IsMock() const { return fMockDb; } /** * Verify that database file strFile is OK. If it is not, diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 285b0099c2..6abd060714 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -77,12 +77,12 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin vErrors.clear(); bumpedTxid.SetNull(); AssertLockHeld(pWallet->cs_wallet); - if (!pWallet->mapWallet.count(txid)) { + auto it = pWallet->mapWallet.find(txid); + if (it == pWallet->mapWallet.end()) { vErrors.push_back("Invalid or non-wallet transaction id"); currentResult = BumpFeeResult::INVALID_ADDRESS_OR_KEY; return; } - auto it = pWallet->mapWallet.find(txid); const CWalletTx& wtx = it->second; if (!preconditionChecks(pWallet, wtx)) { @@ -242,12 +242,13 @@ bool CFeeBumper::commit(CWallet *pWallet) if (!vErrors.empty() || currentResult != BumpFeeResult::OK) { return false; } - if (txid.IsNull() || !pWallet->mapWallet.count(txid)) { + auto it = txid.IsNull() ? pWallet->mapWallet.end() : pWallet->mapWallet.find(txid); + if (it == pWallet->mapWallet.end()) { vErrors.push_back("Invalid or non-wallet transaction id"); currentResult = BumpFeeResult::MISC_ERROR; return false; } - CWalletTx& oldWtx = pWallet->mapWallet[txid]; + CWalletTx& oldWtx = it->second; // make sure the transaction still has no descendants and hasn't been mined in the meantime if (!preconditionChecks(pWallet, oldWtx)) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 30b8c8260a..4ea53c4132 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -281,7 +281,7 @@ UniValue setaccount(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); std::string strAccount; - if (request.params.size() > 1) + if (!request.params[1].isNull()) strAccount = AccountFromValue(request.params[1]); // Only add the account if the address is yours. @@ -463,26 +463,26 @@ UniValue sendtoaddress(const JSONRPCRequest& request) // Wallet comments CWalletTx wtx; - if (request.params.size() > 2 && !request.params[2].isNull() && !request.params[2].get_str().empty()) + if (!request.params[2].isNull() && !request.params[2].get_str().empty()) wtx.mapValue["comment"] = request.params[2].get_str(); - if (request.params.size() > 3 && !request.params[3].isNull() && !request.params[3].get_str().empty()) + if (!request.params[3].isNull() && !request.params[3].get_str().empty()) wtx.mapValue["to"] = request.params[3].get_str(); bool fSubtractFeeFromAmount = false; - if (request.params.size() > 4 && !request.params[4].isNull()) { + if (!request.params[4].isNull()) { fSubtractFeeFromAmount = request.params[4].get_bool(); } CCoinControl coin_control; - if (request.params.size() > 5 && !request.params[5].isNull()) { + if (!request.params[5].isNull()) { coin_control.signalRbf = request.params[5].get_bool(); } - if (request.params.size() > 6 && !request.params[6].isNull()) { + if (!request.params[6].isNull()) { coin_control.m_confirm_target = ParseConfirmTarget(request.params[6]); } - if (request.params.size() > 7 && !request.params[7].isNull()) { + if (!request.params[7].isNull()) { if (!FeeModeFromString(request.params[7].get_str(), coin_control.m_fee_mode)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter"); } @@ -769,18 +769,31 @@ UniValue getbalance(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.params.size() == 0) - return ValueFromAmount(pwallet->GetBalance()); + const UniValue& account_value = request.params[0]; + const UniValue& minconf = request.params[1]; + const UniValue& include_watchonly = request.params[2]; - const std::string& account_param = request.params[0].get_str(); + if (account_value.isNull()) { + if (!minconf.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "getbalance minconf option is only currently supported if an account is specified"); + } + if (!include_watchonly.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "getbalance include_watchonly option is only currently supported if an account is specified"); + } + return ValueFromAmount(pwallet->GetBalance()); + } + + const std::string& account_param = account_value.get_str(); const std::string* account = account_param != "*" ? &account_param : nullptr; int nMinDepth = 1; - if (!request.params[1].isNull()) - nMinDepth = request.params[1].get_int(); + if (!minconf.isNull()) + nMinDepth = minconf.get_int(); isminefilter filter = ISMINE_SPENDABLE; - if(!request.params[2].isNull()) - if(request.params[2].get_bool()) + if(!include_watchonly.isNull()) + if(include_watchonly.get_bool()) filter = filter | ISMINE_WATCH_ONLY; return ValueFromAmount(pwallet->GetLegacyBalance(filter, nMinDepth, account)); @@ -839,11 +852,11 @@ UniValue movecmd(const JSONRPCRequest& request) CAmount nAmount = AmountFromValue(request.params[2]); if (nAmount <= 0) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); - if (request.params.size() > 3) + if (!request.params[3].isNull()) // unused parameter, used to be nMinDepth, keep type-checking it though (void)request.params[3].get_int(); std::string strComment; - if (request.params.size() > 4) + if (!request.params[4].isNull()) strComment = request.params[4].get_str(); if (!pwallet->AccountMove(strFrom, strTo, nAmount, strComment)) { @@ -900,14 +913,14 @@ UniValue sendfrom(const JSONRPCRequest& request) if (nAmount <= 0) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); int nMinDepth = 1; - if (request.params.size() > 3) + if (!request.params[3].isNull()) nMinDepth = request.params[3].get_int(); CWalletTx wtx; wtx.strFromAccount = strAccount; - if (request.params.size() > 4 && !request.params[4].isNull() && !request.params[4].get_str().empty()) + if (!request.params[4].isNull() && !request.params[4].get_str().empty()) wtx.mapValue["comment"] = request.params[4].get_str(); - if (request.params.size() > 5 && !request.params[5].isNull() && !request.params[5].get_str().empty()) + if (!request.params[5].isNull() && !request.params[5].get_str().empty()) wtx.mapValue["to"] = request.params[5].get_str(); EnsureWalletIsUnlocked(pwallet); @@ -987,23 +1000,23 @@ UniValue sendmany(const JSONRPCRequest& request) CWalletTx wtx; wtx.strFromAccount = strAccount; - if (request.params.size() > 3 && !request.params[3].isNull() && !request.params[3].get_str().empty()) + if (!request.params[3].isNull() && !request.params[3].get_str().empty()) wtx.mapValue["comment"] = request.params[3].get_str(); UniValue subtractFeeFromAmount(UniValue::VARR); - if (request.params.size() > 4 && !request.params[4].isNull()) + if (!request.params[4].isNull()) subtractFeeFromAmount = request.params[4].get_array(); CCoinControl coin_control; - if (request.params.size() > 5 && !request.params[5].isNull()) { + if (!request.params[5].isNull()) { coin_control.signalRbf = request.params[5].get_bool(); } - if (request.params.size() > 6 && !request.params[6].isNull()) { + if (!request.params[6].isNull()) { coin_control.m_confirm_target = ParseConfirmTarget(request.params[6]); } - if (request.params.size() > 7 && !request.params[7].isNull()) { + if (!request.params[7].isNull()) { if (!FeeModeFromString(request.params[7].get_str(), coin_control.m_fee_mode)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter"); } @@ -1106,7 +1119,7 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); std::string strAccount; - if (request.params.size() > 2) + if (!request.params[2].isNull()) strAccount = AccountFromValue(request.params[2]); // Construct using pay-to-script-hash: @@ -1124,7 +1137,7 @@ public: CWallet * const pwallet; CScriptID result; - Witnessifier(CWallet *_pwallet) : pwallet(_pwallet) {} + explicit Witnessifier(CWallet *_pwallet) : pwallet(_pwallet) {} bool operator()(const CNoDestination &dest) const { return false; } @@ -1135,7 +1148,7 @@ public: SignatureData sigs; // This check is to make sure that the script we created can actually be solved for and signed by us // if we were to have the private keys. This is just to make sure that the script is valid and that, - // if found in a transaction, we would still accept and relay that transcation. + // if found in a transaction, we would still accept and relay that transaction. if (!ProduceSignature(DummySignatureCreator(pwallet), witscript, sigs) || !VerifyScript(sigs.scriptSig, witscript, &sigs.scriptWitness, MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, DummySignatureCreator(pwallet).Checker())) { return false; @@ -1160,7 +1173,7 @@ public: SignatureData sigs; // This check is to make sure that the script we created can actually be solved for and signed by us // if we were to have the private keys. This is just to make sure that the script is valid and that, - // if found in a transaction, we would still accept and relay that transcation. + // if found in a transaction, we would still accept and relay that transaction. if (!ProduceSignature(DummySignatureCreator(pwallet), witscript, sigs) || !VerifyScript(sigs.scriptSig, witscript, &sigs.scriptWitness, MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, DummySignatureCreator(pwallet).Checker())) { return false; @@ -1645,10 +1658,10 @@ UniValue listtransactions(const JSONRPCRequest& request) for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { CWalletTx *const pwtx = (*it).second.first; - if (pwtx != 0) + if (pwtx != nullptr) ListTransactions(pwallet, *pwtx, strAccount, 0, true, ret, filter); CAccountingEntry *const pacentry = (*it).second.second; - if (pacentry != 0) + if (pacentry != nullptr) AcentryToJSON(*pacentry, strAccount, ret); if ((int)ret.size() >= (nCount+nFrom)) break; @@ -1712,10 +1725,10 @@ UniValue listaccounts(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); int nMinDepth = 1; - if (request.params.size() > 0) + if (!request.params[0].isNull()) nMinDepth = request.params[0].get_int(); isminefilter includeWatchonly = ISMINE_SPENDABLE; - if(request.params.size() > 1) + if(!request.params[1].isNull()) if(request.params[1].get_bool()) includeWatchonly = includeWatchonly | ISMINE_WATCH_ONLY; @@ -1875,10 +1888,11 @@ UniValue listsinceblock(const JSONRPCRequest& request) throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); } for (const CTransactionRef& tx : block.vtx) { - if (pwallet->mapWallet.count(tx->GetHash()) > 0) { + auto it = pwallet->mapWallet.find(tx->GetHash()); + if (it != pwallet->mapWallet.end()) { // We want all transactions regardless of confirmation count to appear here, // even negative confirmation ones, hence the big negative. - ListTransactions(pwallet, pwallet->mapWallet[tx->GetHash()], "*", -100000000, true, removed, filter); + ListTransactions(pwallet, it->second, "*", -100000000, true, removed, filter); } } paltindex = paltindex->pprev; @@ -1958,10 +1972,11 @@ UniValue gettransaction(const JSONRPCRequest& request) filter = filter | ISMINE_WATCH_ONLY; UniValue entry(UniValue::VOBJ); - if (!pwallet->mapWallet.count(hash)) { + auto it = pwallet->mapWallet.find(hash); + if (it == pwallet->mapWallet.end()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id"); } - const CWalletTx& wtx = pwallet->mapWallet[hash]; + const CWalletTx& wtx = it->second; CAmount nCredit = wtx.GetCredit(filter); CAmount nDebit = wtx.GetDebit(filter); @@ -2362,19 +2377,18 @@ UniValue lockunspent(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.params.size() == 1) - RPCTypeCheck(request.params, {UniValue::VBOOL}); - else - RPCTypeCheck(request.params, {UniValue::VBOOL, UniValue::VARR}); + RPCTypeCheckArgument(request.params[0], UniValue::VBOOL); bool fUnlock = request.params[0].get_bool(); - if (request.params.size() == 1) { + if (request.params[1].isNull()) { if (fUnlock) pwallet->UnlockAllCoins(); return true; } + RPCTypeCheckArgument(request.params[1], UniValue::VARR); + UniValue outputs = request.params[1].get_array(); for (unsigned int idx = 0; idx < outputs.size(); idx++) { const UniValue& output = outputs[idx]; @@ -2671,19 +2685,19 @@ UniValue listunspent(const JSONRPCRequest& request) ); int nMinDepth = 1; - if (request.params.size() > 0 && !request.params[0].isNull()) { + if (!request.params[0].isNull()) { RPCTypeCheckArgument(request.params[0], UniValue::VNUM); nMinDepth = request.params[0].get_int(); } int nMaxDepth = 9999999; - if (request.params.size() > 1 && !request.params[1].isNull()) { + if (!request.params[1].isNull()) { RPCTypeCheckArgument(request.params[1], UniValue::VNUM); nMaxDepth = request.params[1].get_int(); } std::set<CBitcoinAddress> setAddress; - if (request.params.size() > 2 && !request.params[2].isNull()) { + if (!request.params[2].isNull()) { RPCTypeCheckArgument(request.params[2], UniValue::VARR); UniValue inputs = request.params[2].get_array(); for (unsigned int idx = 0; idx < inputs.size(); idx++) { @@ -2698,7 +2712,7 @@ UniValue listunspent(const JSONRPCRequest& request) } bool include_unsafe = true; - if (request.params.size() > 3 && !request.params[3].isNull()) { + if (!request.params[3].isNull()) { RPCTypeCheckArgument(request.params[3], UniValue::VBOOL); include_unsafe = request.params[3].get_bool(); } @@ -3113,7 +3127,7 @@ UniValue generate(const JSONRPCRequest& request) int num_generate = request.params[0].get_int(); uint64_t max_tries = 1000000; - if (request.params.size() > 1 && !request.params[1].isNull()) { + if (!request.params[1].isNull()) { max_tries = request.params[1].get_int(); } diff --git a/src/wallet/test/crypto_tests.cpp b/src/wallet/test/crypto_tests.cpp index 98d957b322..cbd74b6f96 100644 --- a/src/wallet/test/crypto_tests.cpp +++ b/src/wallet/test/crypto_tests.cpp @@ -9,86 +9,9 @@ #include <vector> #include <boost/test/unit_test.hpp> -#include <openssl/aes.h> -#include <openssl/evp.h> BOOST_FIXTURE_TEST_SUITE(wallet_crypto, BasicTestingSetup) -bool OldSetKeyFromPassphrase(const SecureString& strKeyData, const std::vector<unsigned char>& chSalt, const unsigned int nRounds, const unsigned int nDerivationMethod, unsigned char* chKey, unsigned char* chIV) -{ - if (nRounds < 1 || chSalt.size() != WALLET_CRYPTO_SALT_SIZE) - return false; - - int i = 0; - if (nDerivationMethod == 0) - i = EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha512(), &chSalt[0], - (unsigned char *)&strKeyData[0], strKeyData.size(), nRounds, chKey, chIV); - - if (i != (int)WALLET_CRYPTO_KEY_SIZE) - { - memory_cleanse(chKey, WALLET_CRYPTO_KEY_SIZE); - memory_cleanse(chIV, WALLET_CRYPTO_IV_SIZE); - return false; - } - return true; -} - -bool OldEncrypt(const CKeyingMaterial& vchPlaintext, std::vector<unsigned char> &vchCiphertext, const unsigned char chKey[32], const unsigned char chIV[16]) -{ - // max ciphertext len for a n bytes of plaintext is - // n + AES_BLOCK_SIZE - 1 bytes - int nLen = vchPlaintext.size(); - int nCLen = nLen + AES_BLOCK_SIZE, nFLen = 0; - vchCiphertext = std::vector<unsigned char> (nCLen); - - EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); - - if (!ctx) return false; - - bool fOk = true; - - EVP_CIPHER_CTX_init(ctx); - if (fOk) fOk = EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), nullptr, chKey, chIV) != 0; - if (fOk) fOk = EVP_EncryptUpdate(ctx, &vchCiphertext[0], &nCLen, &vchPlaintext[0], nLen) != 0; - if (fOk) fOk = EVP_EncryptFinal_ex(ctx, (&vchCiphertext[0]) + nCLen, &nFLen) != 0; - EVP_CIPHER_CTX_cleanup(ctx); - - EVP_CIPHER_CTX_free(ctx); - - if (!fOk) return false; - - vchCiphertext.resize(nCLen + nFLen); - return true; -} - -bool OldDecrypt(const std::vector<unsigned char>& vchCiphertext, CKeyingMaterial& vchPlaintext, const unsigned char chKey[32], const unsigned char chIV[16]) -{ - // plaintext will always be equal to or lesser than length of ciphertext - int nLen = vchCiphertext.size(); - int nPLen = nLen, nFLen = 0; - - vchPlaintext = CKeyingMaterial(nPLen); - - EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); - - if (!ctx) return false; - - bool fOk = true; - - EVP_CIPHER_CTX_init(ctx); - if (fOk) fOk = EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), nullptr, chKey, chIV) != 0; - if (fOk) fOk = EVP_DecryptUpdate(ctx, &vchPlaintext[0], &nPLen, &vchCiphertext[0], nLen) != 0; - if (fOk) fOk = EVP_DecryptFinal_ex(ctx, (&vchPlaintext[0]) + nPLen, &nFLen) != 0; - EVP_CIPHER_CTX_cleanup(ctx); - - EVP_CIPHER_CTX_free(ctx); - - if (!fOk) return false; - - vchPlaintext.resize(nPLen + nFLen); - return true; -} - class TestCrypter { public: @@ -96,25 +19,15 @@ static void TestPassphraseSingle(const std::vector<unsigned char>& vchSalt, cons const std::vector<unsigned char>& correctKey = std::vector<unsigned char>(), const std::vector<unsigned char>& correctIV=std::vector<unsigned char>()) { - unsigned char chKey[WALLET_CRYPTO_KEY_SIZE]; - unsigned char chIV[WALLET_CRYPTO_IV_SIZE]; - CCrypter crypt; crypt.SetKeyFromPassphrase(passphrase, vchSalt, rounds, 0); - OldSetKeyFromPassphrase(passphrase, vchSalt, rounds, 0, chKey, chIV); - - BOOST_CHECK_MESSAGE(memcmp(chKey, crypt.vchKey.data(), crypt.vchKey.size()) == 0, \ - HexStr(chKey, chKey+sizeof(chKey)) + std::string(" != ") + HexStr(crypt.vchKey)); - BOOST_CHECK_MESSAGE(memcmp(chIV, crypt.vchIV.data(), crypt.vchIV.size()) == 0, \ - HexStr(chIV, chIV+sizeof(chIV)) + std::string(" != ") + HexStr(crypt.vchIV)); - if(!correctKey.empty()) - BOOST_CHECK_MESSAGE(memcmp(chKey, &correctKey[0], sizeof(chKey)) == 0, \ - HexStr(chKey, chKey+sizeof(chKey)) + std::string(" != ") + HexStr(correctKey.begin(), correctKey.end())); + BOOST_CHECK_MESSAGE(memcmp(crypt.vchKey.data(), correctKey.data(), crypt.vchKey.size()) == 0, \ + HexStr(crypt.vchKey.begin(), crypt.vchKey.end()) + std::string(" != ") + HexStr(correctKey.begin(), correctKey.end())); if(!correctIV.empty()) - BOOST_CHECK_MESSAGE(memcmp(chIV, &correctIV[0], sizeof(chIV)) == 0, - HexStr(chIV, chIV+sizeof(chIV)) + std::string(" != ") + HexStr(correctIV.begin(), correctIV.end())); + BOOST_CHECK_MESSAGE(memcmp(crypt.vchIV.data(), correctIV.data(), crypt.vchIV.size()) == 0, + HexStr(crypt.vchIV.begin(), crypt.vchIV.end()) + std::string(" != ") + HexStr(correctIV.begin(), correctIV.end())); } static void TestPassphrase(const std::vector<unsigned char>& vchSalt, const SecureString& passphrase, uint32_t rounds, @@ -126,50 +39,26 @@ static void TestPassphrase(const std::vector<unsigned char>& vchSalt, const Secu TestPassphraseSingle(vchSalt, SecureString(i, passphrase.end()), rounds); } - static void TestDecrypt(const CCrypter& crypt, const std::vector<unsigned char>& vchCiphertext, \ const std::vector<unsigned char>& vchPlaintext = std::vector<unsigned char>()) { - CKeyingMaterial vchDecrypted1; - CKeyingMaterial vchDecrypted2; - int result1, result2; - result1 = crypt.Decrypt(vchCiphertext, vchDecrypted1); - result2 = OldDecrypt(vchCiphertext, vchDecrypted2, crypt.vchKey.data(), crypt.vchIV.data()); - BOOST_CHECK(result1 == result2); - - // These two should be equal. However, OpenSSL 1.0.1j introduced a change - // that would zero all padding except for the last byte for failed decrypts. - // This behavior was reverted for 1.0.1k. - if (vchDecrypted1 != vchDecrypted2 && vchDecrypted1.size() >= AES_BLOCK_SIZE && SSLeay() == 0x100010afL) - { - for(CKeyingMaterial::iterator it = vchDecrypted1.end() - AES_BLOCK_SIZE; it != vchDecrypted1.end() - 1; it++) - *it = 0; - } - - BOOST_CHECK_MESSAGE(vchDecrypted1 == vchDecrypted2, HexStr(vchDecrypted1.begin(), vchDecrypted1.end()) + " != " + HexStr(vchDecrypted2.begin(), vchDecrypted2.end())); - + CKeyingMaterial vchDecrypted; + crypt.Decrypt(vchCiphertext, vchDecrypted); if (vchPlaintext.size()) - BOOST_CHECK(CKeyingMaterial(vchPlaintext.begin(), vchPlaintext.end()) == vchDecrypted2); + BOOST_CHECK(CKeyingMaterial(vchPlaintext.begin(), vchPlaintext.end()) == vchDecrypted); } static void TestEncryptSingle(const CCrypter& crypt, const CKeyingMaterial& vchPlaintext, const std::vector<unsigned char>& vchCiphertextCorrect = std::vector<unsigned char>()) { - std::vector<unsigned char> vchCiphertext1; - std::vector<unsigned char> vchCiphertext2; - int result1 = crypt.Encrypt(vchPlaintext, vchCiphertext1); - - int result2 = OldEncrypt(vchPlaintext, vchCiphertext2, crypt.vchKey.data(), crypt.vchIV.data()); - BOOST_CHECK(result1 == result2); - BOOST_CHECK(vchCiphertext1 == vchCiphertext2); + std::vector<unsigned char> vchCiphertext; + crypt.Encrypt(vchPlaintext, vchCiphertext); if (!vchCiphertextCorrect.empty()) - BOOST_CHECK(vchCiphertext2 == vchCiphertextCorrect); + BOOST_CHECK(vchCiphertext == vchCiphertextCorrect); const std::vector<unsigned char> vchPlaintext2(vchPlaintext.begin(), vchPlaintext.end()); - - if(vchCiphertext1 == vchCiphertext2) - TestDecrypt(crypt, vchCiphertext1, vchPlaintext2); + TestDecrypt(crypt, vchCiphertext, vchPlaintext2); } static void TestEncrypt(const CCrypter& crypt, const std::vector<unsigned char>& vchPlaintextIn, \ diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 97a6d98397..9373b7907c 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -10,7 +10,7 @@ /** Testing setup and teardown for wallet. */ struct WalletTestingSetup: public TestingSetup { - WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); + explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e2f12fa024..c926f0159f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -567,8 +567,9 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) void CWallet::AddToSpends(const uint256& wtxid) { - assert(mapWallet.count(wtxid)); - CWalletTx& thisTx = mapWallet[wtxid]; + auto it = mapWallet.find(wtxid); + assert(it != mapWallet.end()); + CWalletTx& thisTx = it->second; if (thisTx.IsCoinBase()) // Coinbases don't spend anything! return; @@ -683,13 +684,13 @@ DBErrors CWallet::ReorderTransactions() for (std::map<uint256, CWalletTx>::iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { CWalletTx* wtx = &((*it).second); - txByTime.insert(std::make_pair(wtx->nTimeReceived, TxPair(wtx, (CAccountingEntry*)0))); + txByTime.insert(std::make_pair(wtx->nTimeReceived, TxPair(wtx, nullptr))); } std::list<CAccountingEntry> acentries; walletdb.ListAccountCreditDebit("", acentries); for (CAccountingEntry& entry : acentries) { - txByTime.insert(std::make_pair(entry.nTime, TxPair((CWalletTx*)0, &entry))); + txByTime.insert(std::make_pair(entry.nTime, TxPair(nullptr, &entry))); } nOrderPosNext = 0; @@ -698,7 +699,7 @@ DBErrors CWallet::ReorderTransactions() { CWalletTx *const pwtx = (*it).second.first; CAccountingEntry *const pacentry = (*it).second.second; - int64_t& nOrderPos = (pwtx != 0) ? pwtx->nOrderPos : pacentry->nOrderPos; + int64_t& nOrderPos = (pwtx != nullptr) ? pwtx->nOrderPos : pacentry->nOrderPos; if (nOrderPos == -1) { @@ -883,7 +884,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) { wtx.nTimeReceived = GetAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&walletdb); - wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0))); + wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr))); wtx.nTimeSmart = ComputeTimeSmart(wtx); AddToSpends(hash); } @@ -948,11 +949,12 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn) mapWallet[hash] = wtxIn; CWalletTx& wtx = mapWallet[hash]; wtx.BindWallet(this); - wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0))); + wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr))); AddToSpends(hash); for (const CTxIn& txin : wtx.tx->vin) { - if (mapWallet.count(txin.prevout.hash)) { - CWalletTx& prevtx = mapWallet[txin.prevout.hash]; + auto it = mapWallet.find(txin.prevout.hash); + if (it != mapWallet.end()) { + CWalletTx& prevtx = it->second; if (prevtx.nIndex == -1 && !prevtx.hashUnset()) { MarkConflicted(prevtx.hashBlock, wtx.GetHash()); } @@ -1051,8 +1053,9 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) std::set<uint256> done; // Can't mark abandoned if confirmed or in mempool - assert(mapWallet.count(hashTx)); - CWalletTx& origtx = mapWallet[hashTx]; + auto it = mapWallet.find(hashTx); + assert(it != mapWallet.end()); + CWalletTx& origtx = it->second; if (origtx.GetDepthInMainChain() > 0 || origtx.InMempool()) { return false; } @@ -1063,8 +1066,9 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) uint256 now = *todo.begin(); todo.erase(now); done.insert(now); - assert(mapWallet.count(now)); - CWalletTx& wtx = mapWallet[now]; + auto it = mapWallet.find(now); + assert(it != mapWallet.end()); + CWalletTx& wtx = it->second; int currentconfirm = wtx.GetDepthInMainChain(); // If the orig tx was not in block, none of its spends can be assert(currentconfirm <= 0); @@ -1089,8 +1093,10 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) // available of the outputs it spends. So force those to be recomputed for (const CTxIn& txin : wtx.tx->vin) { - if (mapWallet.count(txin.prevout.hash)) - mapWallet[txin.prevout.hash].MarkDirty(); + auto it = mapWallet.find(txin.prevout.hash); + if (it != mapWallet.end()) { + it->second.MarkDirty(); + } } } } @@ -1128,8 +1134,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) uint256 now = *todo.begin(); todo.erase(now); done.insert(now); - assert(mapWallet.count(now)); - CWalletTx& wtx = mapWallet[now]; + auto it = mapWallet.find(now); + assert(it != mapWallet.end()); + CWalletTx& wtx = it->second; int currentconfirm = wtx.GetDepthInMainChain(); if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. @@ -1148,10 +1155,11 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } // If a transaction changes 'conflicted' state, that changes the balance // available of the outputs it spends. So force those to be recomputed - for (const CTxIn& txin : wtx.tx->vin) - { - if (mapWallet.count(txin.prevout.hash)) - mapWallet[txin.prevout.hash].MarkDirty(); + for (const CTxIn& txin : wtx.tx->vin) { + auto it = mapWallet.find(txin.prevout.hash); + if (it != mapWallet.end()) { + it->second.MarkDirty(); + } } } } @@ -1166,10 +1174,11 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin // If a transaction changes 'conflicted' state, that changes the balance // available of the outputs it spends. So force those to be // recomputed, also: - for (const CTxIn& txin : tx.vin) - { - if (mapWallet.count(txin.prevout.hash)) - mapWallet[txin.prevout.hash].MarkDirty(); + for (const CTxIn& txin : tx.vin) { + auto it = mapWallet.find(txin.prevout.hash); + if (it != mapWallet.end()) { + it->second.MarkDirty(); + } } } @@ -1738,7 +1747,7 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const CAmount CWalletTx::GetAvailableCredit(bool fUseCache) const { - if (pwallet == 0) + if (pwallet == nullptr) return 0; // Must wait until coinbase is safely deep enough in the chain before valuing it @@ -1782,7 +1791,7 @@ CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool& fUseCache) const CAmount CWalletTx::GetAvailableWatchOnlyCredit(const bool& fUseCache) const { - if (pwallet == 0) + if (pwallet == nullptr) return 0; // Must wait until coinbase is safely deep enough in the chain before valuing it @@ -2515,7 +2524,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC if (nChangePosInOut != -1) { tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]); - // we dont have the normal Create/Commit cycle, and dont want to risk reusing change, + // we don't have the normal Create/Commit cycle, and don't want to risk reusing change, // so just remove the key from the keypool here. reservekey.KeepKey(); } @@ -2959,7 +2968,7 @@ bool CWallet::AddAccountingEntry(const CAccountingEntry& acentry, CWalletDB *pwa laccentries.push_back(acentry); CAccountingEntry & entry = laccentries.back(); - wtxOrdered.insert(std::make_pair(entry.nOrderPos, TxPair((CWalletTx*)0, &entry))); + wtxOrdered.insert(std::make_pair(entry.nOrderPos, TxPair(nullptr, &entry))); return true; } @@ -2982,9 +2991,11 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) } } + // This wallet is in its first run if all of these are empty + fFirstRunRet = mapKeys.empty() && mapCryptedKeys.empty() && mapWatchKeys.empty() && setWatchOnly.empty() && mapScripts.empty(); + if (nLoadWalletRet != DB_LOAD_OK) return nLoadWalletRet; - fFirstRunRet = !vchDefaultKey.IsValid(); uiInterface.LoadWallet(this); @@ -2994,7 +3005,6 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) { AssertLockHeld(cs_wallet); // mapWallet - vchDefaultKey = CPubKey(); DBErrors nZapSelectTxRet = CWalletDB(*dbw,"cr+").ZapSelectTx(vHashIn, vHashOut); for (uint256 hash : vHashOut) mapWallet.erase(hash); @@ -3023,7 +3033,6 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 DBErrors CWallet::ZapWalletTx(std::vector<CWalletTx>& vWtx) { - vchDefaultKey = CPubKey(); DBErrors nZapWalletTxRet = CWalletDB(*dbw,"cr+").ZapWalletTx(vWtx); if (nZapWalletTxRet == DB_NEED_REWRITE) { @@ -3099,14 +3108,6 @@ const std::string& CWallet::GetAccountName(const CScript& scriptPubKey) const return DEFAULT_ACCOUNT_NAME; } -bool CWallet::SetDefaultKey(const CPubKey &vchPubKey) -{ - if (!CWalletDB(*dbw).WriteDefaultKey(vchPubKey)) - return false; - vchDefaultKey = vchPubKey; - return true; -} - /** * Mark old keypool keys as used, * and generate all new keys @@ -3527,15 +3528,11 @@ void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id) m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); } walletdb.ErasePool(index); + LogPrintf("keypool index %d removed\n", index); it = setKeyPool->erase(it); } } -bool CWallet::HasUnusedKeys(int min_keys) const -{ - return setExternalKeyPool.size() >= min_keys && (setInternalKeyPool.size() >= min_keys || !CanSupportFeature(FEATURE_HD_SPLIT)); -} - void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script) { std::shared_ptr<CReserveKey> rKey = std::make_shared<CReserveKey>(this); @@ -3765,15 +3762,12 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) uiInterface.InitMessage(_("Zapping all transactions from wallet...")); std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile)); - CWallet *tempWallet = new CWallet(std::move(dbw)); + std::unique_ptr<CWallet> tempWallet(new CWallet(std::move(dbw))); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DB_LOAD_OK) { InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); return nullptr; } - - delete tempWallet; - tempWallet = nullptr; } uiInterface.InitMessage(_("Loading wallet...")); @@ -3842,13 +3836,11 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) if (!walletInstance->SetHDMasterKey(masterPubKey)) throw std::runtime_error(std::string(__func__) + ": Storing master key failed"); } - CPubKey newDefaultKey; - if (walletInstance->GetKeyFromPool(newDefaultKey, false)) { - walletInstance->SetDefaultKey(newDefaultKey); - if (!walletInstance->SetAddressBook(walletInstance->vchDefaultKey.GetID(), "", "receive")) { - InitError(_("Cannot write default address") += "\n"); - return nullptr; - } + + // Top up the keypool + if (!walletInstance->TopUpKeyPool()) { + InitError(_("Unable to generate initial keys") += "\n"); + return NULL; } walletInstance->SetBestChain(chainActive.GetLocator()); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bceeb12fbb..73ad3bdeca 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -208,7 +208,7 @@ public: Init(); } - CMerkleTx(CTransactionRef arg) + explicit CMerkleTx(CTransactionRef arg) { SetTx(std::move(arg)); Init(); @@ -548,7 +548,7 @@ public: //! todo: add something to note what created it (user, getnewaddress, change) //! maybe should have a map<string, string> property map - CWalletKey(int64_t nExpires=0); + explicit CWalletKey(int64_t nExpires=0); ADD_SERIALIZE_METHODS; @@ -651,7 +651,7 @@ private: * 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. */ -class CWallet : public CCryptoKeyStore, public CValidationInterface +class CWallet final : public CCryptoKeyStore, public CValidationInterface { private: static std::atomic<bool> fFlushScheduled; @@ -765,7 +765,7 @@ public: } // Create wallet with passed-in database handle - CWallet(std::unique_ptr<CWalletDBWrapper> dbw_in) : dbw(std::move(dbw_in)) + explicit CWallet(std::unique_ptr<CWalletDBWrapper> dbw_in) : dbw(std::move(dbw_in)) { SetNull(); } @@ -807,8 +807,6 @@ public: std::map<CTxDestination, CAddressBookData> mapAddressBook; - CPubKey vchDefaultKey; - std::set<COutPoint> setLockedCoins; const CWalletTx* GetWalletTx(const uint256& hash) const; @@ -974,8 +972,6 @@ public: */ void MarkReserveKeysAsUsed(int64_t keypool_id); const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; } - /** Does the wallet have at least min_keys in the keypool? */ - bool HasUnusedKeys(int min_keys) const; std::set< std::set<CTxDestination> > GetAddressGroupings(); std::map<CTxDestination, CAmount> GetAddressBalances(); @@ -1030,8 +1026,6 @@ public: return setInternalKeyPool.size() + setExternalKeyPool.size(); } - bool SetDefaultKey(const CPubKey &vchPubKey); - //! signify that a particular wallet feature is now used. this may change nWalletVersion and nWalletMaxVersion if those are lower bool SetMinVersion(enum WalletFeature, CWalletDB* pwalletdbIn = nullptr, bool fExplicit = false); @@ -1115,7 +1109,7 @@ public: }; /** A key allocated from the key pool. */ -class CReserveKey : public CReserveScript +class CReserveKey final : public CReserveScript { protected: CWallet* pwallet; @@ -1123,7 +1117,7 @@ protected: CPubKey vchPubKey; bool fInternal; public: - CReserveKey(CWallet* pwalletIn) + explicit CReserveKey(CWallet* pwalletIn) { nIndex = -1; pwallet = pwalletIn; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 72c22d2259..12da3cce64 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -130,11 +130,6 @@ bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext) return WriteIC(std::string("orderposnext"), nOrderPosNext); } -bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey) -{ - return WriteIC(std::string("defaultkey"), vchPubKey); -} - bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) { return batch.Read(std::make_pair(std::string("pool"), nPool), keypool); @@ -452,7 +447,14 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } else if (strType == "defaultkey") { - ssValue >> pwallet->vchDefaultKey; + // We don't want or need the default key, but if there is one set, + // we want to make sure that it is valid so that we can detect corruption + CPubKey vchPubKey; + ssValue >> vchPubKey; + if (!vchPubKey.IsValid()) { + strErr = "Error reading wallet database: Default Key corrupt"; + return false; + } } else if (strType == "pool") { @@ -522,7 +524,6 @@ bool CWalletDB::IsKeyType(const std::string& strType) DBErrors CWalletDB::LoadWallet(CWallet* pwallet) { - pwallet->vchDefaultKey = CPubKey(); CWalletScanState wss; bool fNoncriticalErrors = false; DBErrors result = DB_LOAD_OK; @@ -565,7 +566,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) { // losing keys is considered a catastrophic error, anything else // we assume the user can live with: - if (IsKeyType(strType)) + if (IsKeyType(strType) || strType == "defaultkey") result = DB_CORRUPT; else { @@ -621,7 +622,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) pwallet->laccentries.clear(); ListAccountCreditDebit("*", pwallet->laccentries); for (CAccountingEntry& entry : pwallet->laccentries) { - pwallet->wtxOrdered.insert(make_pair(entry.nOrderPos, CWallet::TxPair((CWalletTx*)0, &entry))); + pwallet->wtxOrdered.insert(make_pair(entry.nOrderPos, CWallet::TxPair(nullptr, &entry))); } return result; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index d78f143ebd..4f8ea185d5 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -105,7 +105,7 @@ public: { SetNull(); } - CKeyMetadata(int64_t nCreateTime_) + explicit CKeyMetadata(int64_t nCreateTime_) { SetNull(); nCreateTime = nCreateTime_; @@ -162,7 +162,7 @@ private: } public: - CWalletDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool _fFlushOnClose = true) : + explicit CWalletDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool _fFlushOnClose = true) : batch(dbw, pszMode, _fFlushOnClose), m_dbw(dbw) { @@ -191,8 +191,6 @@ public: bool WriteOrderPosNext(int64_t nOrderPosNext); - bool WriteDefaultKey(const CPubKey& vchPubKey); - bool ReadPool(int64_t nPool, CKeyPool& keypool); bool WritePool(int64_t nPool, const CKeyPool& keypool); bool ErasePool(int64_t nPool); |