From d78a8dc3e82564ca8e56b81f9f21af9295b013dd Mon Sep 17 00:00:00 2001 From: practicalswift Date: Fri, 27 Jul 2018 08:22:42 +0200 Subject: [PATCH] Return void instead of bool for functions that cannot fail * CBlockTreeDB::ReadReindexing(...) * CChainState::ResetBlockFailureFlags(...) * CTxMemPool::addUnchecked(...) * CWallet::LoadDestData(...) * CWallet::LoadKeyMetadata(...) * CWallet::LoadScriptMetadata(...) * CWallet::LoadToWallet(...) * CWallet::SetHDChain(...) * CWallet::SetHDSeed(...) * RemoveLocal(...) * SetMinVersion(...) * StartHTTPServer(...) * StartRPC(...) * TorControlConnection::Disconnect(...) --- src/httpserver.cpp | 3 +-- src/httpserver.h | 2 +- src/init.cpp | 6 ++---- src/net.cpp | 3 +-- src/net.h | 2 +- src/rpc/server.cpp | 3 +-- src/rpc/server.h | 2 +- src/torcontrol.cpp | 5 ++--- src/txdb.cpp | 3 +-- src/txdb.h | 2 +- src/txmempool.cpp | 6 ++---- src/txmempool.h | 4 ++-- src/validation.cpp | 8 ++++---- src/validation.h | 2 +- src/wallet/wallet.cpp | 38 +++++++++++--------------------------- src/wallet/wallet.h | 14 +++++++------- src/wallet/walletdb.cpp | 12 ++---------- 17 files changed, 41 insertions(+), 74 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index bd08b04c0..3a3a26b7d 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -423,7 +423,7 @@ std::thread threadHTTP; std::future threadResult; static std::vector g_thread_http_workers; -bool StartHTTPServer() +void StartHTTPServer() { LogPrint(BCLog::HTTP, "Starting HTTP server\n"); int rpcThreads = std::max((long)gArgs.GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L); @@ -435,7 +435,6 @@ bool StartHTTPServer() for (int i = 0; i < rpcThreads; i++) { g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue); } - return true; } void InterruptHTTPServer() diff --git a/src/httpserver.h b/src/httpserver.h index 8132c887b..8a3adaf91 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -26,7 +26,7 @@ bool InitHTTPServer(); * This is separate from InitHTTPServer to give users race-condition-free time * to register their handlers between InitHTTPServer and StartHTTPServer. */ -bool StartHTTPServer(); +void StartHTTPServer(); /** Interrupt HTTP server threads */ void InterruptHTTPServer(); /** Stop HTTP server */ diff --git a/src/init.cpp b/src/init.cpp index c7ea40817..36b094df0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -731,14 +731,12 @@ static bool AppInitServers() RPCServer::OnStopped(&OnRPCStopped); if (!InitHTTPServer()) return false; - if (!StartRPC()) - return false; + StartRPC(); if (!StartHTTPRPC()) return false; if (gArgs.GetBoolArg("-rest", DEFAULT_REST_ENABLE) && !StartREST()) return false; - if (!StartHTTPServer()) - return false; + StartHTTPServer(); return true; } diff --git a/src/net.cpp b/src/net.cpp index 67a73fa9f..0ebfefa75 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -235,12 +235,11 @@ bool AddLocal(const CNetAddr &addr, int nScore) return AddLocal(CService(addr, GetListenPort()), nScore); } -bool RemoveLocal(const CService& addr) +void RemoveLocal(const CService& addr) { LOCK(cs_mapLocalHost); LogPrintf("RemoveLocal(%s)\n", addr.ToString()); mapLocalHost.erase(addr); - return true; } /** Make a particular network entirely off-limits (no automatic connects to it) */ diff --git a/src/net.h b/src/net.h index 607b2fc53..36c2a4b8f 100644 --- a/src/net.h +++ b/src/net.h @@ -505,7 +505,7 @@ bool IsLimited(enum Network net); bool IsLimited(const CNetAddr& addr); bool AddLocal(const CService& addr, int nScore = LOCAL_NONE); bool AddLocal(const CNetAddr& addr, int nScore = LOCAL_NONE); -bool RemoveLocal(const CService& addr); +void RemoveLocal(const CService& addr); bool SeenLocal(const CService& addr); bool IsLocal(const CService& addr); bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index b420e9d8b..e46bf2f76 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -301,12 +301,11 @@ bool CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) return true; } -bool StartRPC() +void StartRPC() { LogPrint(BCLog::RPC, "Starting RPC\n"); fRPCRunning = true; g_rpcSignals.Started(); - return true; } void InterruptRPC() diff --git a/src/rpc/server.h b/src/rpc/server.h index 373914885..a9dbfbd21 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -198,7 +198,7 @@ extern CAmount AmountFromValue(const UniValue& value); extern std::string HelpExampleCli(const std::string& methodname, const std::string& args); extern std::string HelpExampleRpc(const std::string& methodname, const std::string& args); -bool StartRPC(); +void StartRPC(); void InterruptRPC(); void StopRPC(); std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq); diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 1f42ab8fa..fbc193c2c 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -91,7 +91,7 @@ public: /** * Disconnect from Tor control port. */ - bool Disconnect(); + void Disconnect(); /** Send a command, register a handler for the reply. * A trailing CRLF is automatically added. @@ -223,12 +223,11 @@ bool TorControlConnection::Connect(const std::string &target, const ConnectionCB return true; } -bool TorControlConnection::Disconnect() +void TorControlConnection::Disconnect() { if (b_conn) bufferevent_free(b_conn); b_conn = nullptr; - return true; } bool TorControlConnection::Command(const std::string &cmd, const ReplyHandlerCB& reply_handler) diff --git a/src/txdb.cpp b/src/txdb.cpp index 1fc791944..cbea55073 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -160,9 +160,8 @@ bool CBlockTreeDB::WriteReindexing(bool fReindexing) { return Erase(DB_REINDEX_FLAG); } -bool CBlockTreeDB::ReadReindexing(bool &fReindexing) { +void CBlockTreeDB::ReadReindexing(bool &fReindexing) { fReindexing = Exists(DB_REINDEX_FLAG); - return true; } bool CBlockTreeDB::ReadLastBlockFile(int &nFile) { diff --git a/src/txdb.h b/src/txdb.h index 100adb428..84011097c 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -92,7 +92,7 @@ public: bool ReadBlockFileInfo(int nFile, CBlockFileInfo &info); bool ReadLastBlockFile(int &nFile); bool WriteReindexing(bool fReindexing); - bool ReadReindexing(bool &fReindexing); + void ReadReindexing(bool &fReindexing); bool WriteFlag(const std::string &name, bool fValue); bool ReadFlag(const std::string &name, bool &fValue); bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function insertBlockIndex); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 2108e6ba2..9d705e3d2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -357,7 +357,7 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n) nTransactionsUpdated += n; } -bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate) +void CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate) { NotifyEntryAdded(entry.GetSharedTx()); // Add to memory pool without checking anything. @@ -412,8 +412,6 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, vTxHashes.emplace_back(tx.GetWitnessHash(), newit); newit->vTxHashesIdx = vTxHashes.size() - 1; - - return true; } void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) @@ -933,7 +931,7 @@ int CTxMemPool::Expire(int64_t time) { return stage.size(); } -bool CTxMemPool::addUnchecked(const uint256&hash, const CTxMemPoolEntry &entry, bool validFeeEstimate) +void CTxMemPool::addUnchecked(const uint256&hash, const CTxMemPoolEntry &entry, bool validFeeEstimate) { LOCK(cs); setEntries setAncestors; diff --git a/src/txmempool.h b/src/txmempool.h index f3b8a68a9..bb676cf05 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -539,8 +539,8 @@ public: // Note that addUnchecked is ONLY called from ATMP outside of tests // and any other callers may break wallet's in-mempool tracking (due to // lack of CValidationInterface::TransactionAddedToMempool callbacks). - bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool validFeeEstimate = true); - bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate = true); + void addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool validFeeEstimate = true); + void addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate = true); void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); diff --git a/src/validation.cpp b/src/validation.cpp index f1b63b5b4..702a8d7e0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -178,7 +178,7 @@ public: // Manual block validity manipulation: bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool ReplayBlocks(const CChainParams& params, CCoinsView* view); bool RewindBlockIndex(const CChainParams& params); @@ -2882,7 +2882,7 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C return g_chainstate.InvalidateBlock(state, chainparams, pindex); } -bool CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { +void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { AssertLockHeld(cs_main); int nHeight = pindex->nHeight; @@ -2914,9 +2914,9 @@ bool CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { } pindex = pindex->pprev; } - return true; } -bool ResetBlockFailureFlags(CBlockIndex *pindex) { + +void ResetBlockFailureFlags(CBlockIndex *pindex) { return g_chainstate.ResetBlockFailureFlags(pindex); } diff --git a/src/validation.h b/src/validation.h index 1aff10f02..ce46a5a78 100644 --- a/src/validation.h +++ b/src/validation.h @@ -449,7 +449,7 @@ bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIn bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Remove invalidity status from a block and its descendants. */ -bool ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** The currently-connected chain of blocks (protected by cs_main). */ extern CChain& chainActive; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3ec6aefae..15139c981 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -304,20 +304,18 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, } } -bool CWallet::LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &meta) +void CWallet::LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &meta) { AssertLockHeld(cs_wallet); // mapKeyMetadata UpdateTimeFirstKey(meta.nCreateTime); mapKeyMetadata[keyID] = meta; - return true; } -bool CWallet::LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &meta) +void CWallet::LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &meta) { AssertLockHeld(cs_wallet); // m_script_metadata UpdateTimeFirstKey(meta.nCreateTime); m_script_metadata[script_id] = meta; - return true; } bool CWallet::LoadCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret) @@ -470,11 +468,11 @@ void CWallet::ChainStateFlushed(const CBlockLocator& loc) batch.WriteBestBlock(loc); } -bool CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in, bool fExplicit) +void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in, bool fExplicit) { LOCK(cs_wallet); // nWalletVersion if (nWalletVersion >= nVersion) - return true; + return; // when doing an explicit upgrade, if we pass the max version permitted, upgrade all the way if (fExplicit && nVersion > nWalletMaxVersion) @@ -492,8 +490,6 @@ bool CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in, if (!batch_in) delete batch; } - - return true; } bool CWallet::SetMaxVersion(int nVersion) @@ -703,9 +699,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // if we are using HD, replace the HD seed with a new one if (IsHDEnabled()) { - if (!SetHDSeed(GenerateNewSeed())) { - return false; - } + SetHDSeed(GenerateNewSeed()); } NewKeyPool(); @@ -1006,7 +1000,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) return true; } -bool CWallet::LoadToWallet(const CWalletTx& wtxIn) +void CWallet::LoadToWallet(const CWalletTx& wtxIn) { uint256 hash = wtxIn.GetHash(); const auto& ins = mapWallet.emplace(hash, wtxIn); @@ -1025,8 +1019,6 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn) } } } - - return true; } bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate) @@ -1478,7 +1470,7 @@ CPubKey CWallet::DeriveNewSeed(const CKey& key) return seed; } -bool CWallet::SetHDSeed(const CPubKey& seed) +void CWallet::SetHDSeed(const CPubKey& seed) { LOCK(cs_wallet); // store the keyid (hash160) together with @@ -1488,18 +1480,15 @@ bool CWallet::SetHDSeed(const CPubKey& seed) newHdChain.nVersion = CanSupportFeature(FEATURE_HD_SPLIT) ? CHDChain::VERSION_HD_CHAIN_SPLIT : CHDChain::VERSION_HD_BASE; newHdChain.seed_id = seed.GetID(); SetHDChain(newHdChain, false); - - return true; } -bool CWallet::SetHDChain(const CHDChain& chain, bool memonly) +void CWallet::SetHDChain(const CHDChain& chain, bool memonly) { LOCK(cs_wallet); if (!memonly && !WalletBatch(*database).WriteHDChain(chain)) throw std::runtime_error(std::string(__func__) + ": writing chain failed"); hdChain = chain; - return true; } bool CWallet::IsHDEnabled() const @@ -3899,10 +3888,9 @@ bool CWallet::EraseDestData(const CTxDestination &dest, const std::string &key) return WalletBatch(*database).EraseDestData(EncodeDestination(dest), key); } -bool CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value) +void CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value) { mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); - return true; } bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const @@ -4091,9 +4079,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, // generate a new master key CPubKey masterPubKey = walletInstance->GenerateNewSeed(); - if (!walletInstance->SetHDSeed(masterPubKey)) { - throw std::runtime_error(std::string(__func__) + ": Storing master key failed"); - } + walletInstance->SetHDSeed(masterPubKey); hd_upgrade = true; } // Upgrade to HD chain split if necessary @@ -4130,9 +4116,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, } else { // generate a new seed CPubKey seed = walletInstance->GenerateNewSeed(); - if (!walletInstance->SetHDSeed(seed)) { - throw std::runtime_error(std::string(__func__) + ": Storing HD seed failed"); - } + walletInstance->SetHDSeed(seed); } // Top up the keypool diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2ada23351..9f9501cf7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -885,8 +885,8 @@ public: //! Adds a key to the store, without saving it to disk (used by LoadWallet) bool LoadKey(const CKey& key, const CPubKey &pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); } //! Load metadata (used by LoadWallet) - bool LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &metadata) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &metadata) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &metadata) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &metadata) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; } void UpdateTimeFirstKey(int64_t nCreateTime) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -903,7 +903,7 @@ public: //! Erases a destination data tuple in the store and on disk bool EraseDestData(const CTxDestination &dest, const std::string &key); //! Adds a destination data tuple to the store, without saving it to disk - bool LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value); + void LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value); //! Look up a destination data tuple in the store, return true if found false otherwise bool GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const; //! Get all destination values matching a prefix. @@ -936,7 +936,7 @@ public: void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); - bool LoadToWallet(const CWalletTx& wtxIn); + void LoadToWallet(const CWalletTx& wtxIn); void TransactionAddedToMempool(const CTransactionRef& tx) override; void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; void BlockDisconnected(const std::shared_ptr& pblock) override; @@ -1075,7 +1075,7 @@ public: } //! signify that a particular wallet feature is now used. this may change nWalletVersion and nWalletMaxVersion if those are lower - bool SetMinVersion(enum WalletFeature, WalletBatch* batch_in = nullptr, bool fExplicit = false); + void SetMinVersion(enum WalletFeature, WalletBatch* batch_in = nullptr, bool fExplicit = false); //! change which version we're allowed to upgrade to (note that this does not immediately imply upgrading to that format) bool SetMaxVersion(int nVersion); @@ -1146,7 +1146,7 @@ public: bool BackupWallet(const std::string& strDest); /* Set the HD chain model (chain child index counters) */ - bool SetHDChain(const CHDChain& chain, bool memonly); + void SetHDChain(const CHDChain& chain, bool memonly); const CHDChain& GetHDChain() const { return hdChain; } /* Returns true if HD is enabled */ @@ -1162,7 +1162,7 @@ public: Sets the seed's version based on the current wallet version (so the caller must ensure the current wallet version is correct before calling this function). */ - bool SetHDSeed(const CPubKey& key); + void SetHDSeed(const CPubKey& key); /** * Blocks until the wallet state is up-to-date to /at least/ the current diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 67fcaa725..43e474731 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -495,21 +495,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, ssKey >> strAddress; ssKey >> strKey; ssValue >> strValue; - if (!pwallet->LoadDestData(DecodeDestination(strAddress), strKey, strValue)) - { - strErr = "Error reading wallet database: LoadDestData failed"; - return false; - } + pwallet->LoadDestData(DecodeDestination(strAddress), strKey, strValue); } else if (strType == "hdchain") { CHDChain chain; ssValue >> chain; - if (!pwallet->SetHDChain(chain, true)) - { - strErr = "Error reading wallet database: SetHDChain failed"; - return false; - } + pwallet->SetHDChain(chain, true); } else if (strType == "flags") { uint64_t flags; ssValue >> flags;