Skip to content

Commit

Permalink
Return void instead of bool for functions that cannot fail
Browse files Browse the repository at this point in the history
* CBlockTreeDB::ReadReindexing(...)
* CChainState::ResetBlockFailureFlags(...)
* CTxMemPool::addUnchecked(...)
* CWallet::LoadDestData(...)
* CWallet::LoadKeyMetadata(...)
* CWallet::LoadScriptMetadata(...)
* CWallet::LoadToWallet(...)
* CWallet::SetHDChain(...)
* CWallet::SetHDSeed(...)
* RemoveLocal(...)
* SetMinVersion(...)
* StartHTTPServer(...)
* StartRPC(...)
* TorControlConnection::Disconnect(...)
  • Loading branch information
practicalswift committed Jul 27, 2018
1 parent f58674a commit d78a8dc
Show file tree
Hide file tree
Showing 17 changed files with 41 additions and 74 deletions.
3 changes: 1 addition & 2 deletions src/httpserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ std::thread threadHTTP;
std::future<bool> threadResult;
static std::vector<std::thread> 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);
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion src/httpserver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
6 changes: 2 additions & 4 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
Expand Down
2 changes: 1 addition & 1 deletion src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/rpc/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions src/torcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class TorControlConnection
/**
* Disconnect from Tor control port.
*/
bool Disconnect();
void Disconnect();

/** Send a command, register a handler for the reply.
* A trailing CRLF is automatically added.
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions src/txdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/txdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class CBlockTreeDB : public CDBWrapper
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<CBlockIndex*(const uint256&)> insertBlockIndex);
Expand Down
6 changes: 2 additions & 4 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/txmempool.h
Original file line number Diff line number Diff line change
Expand Up @@ -539,8 +539,8 @@ class CTxMemPool
// 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);
Expand Down
8 changes: 4 additions & 4 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class CChainState {
// 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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
38 changes: 11 additions & 27 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<unsigned char> &vchCryptedSecret)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -4091,9 +4079,7 @@ std::shared_ptr<CWallet> 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
Expand Down Expand Up @@ -4130,9 +4116,7 @@ std::shared_ptr<CWallet> 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
Expand Down
Loading

0 comments on commit d78a8dc

Please sign in to comment.