Skip to content

Commit 8aa8f73

Browse files
ryanofskyryanofsky
committed
refactor: Replace std::optional<bilingual_str> with util::Result
1 parent 5f49cb1 commit 8aa8f73

15 files changed

+66
-65
lines changed

‎src/addrdb.cpp‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,10 @@ void ReadFromStream(AddrMan& addr, CDataStream& ssPeers)
183183
DeserializeDB(ssPeers, addr, false);
184184
}
185185

186-
std::optional<bilingual_str> LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args, std::unique_ptr<AddrMan>& addrman)
186+
util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args)
187187
{
188188
auto check_addrman = std::clamp<int32_t>(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
189-
addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);
189+
auto addrman{std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman)};
190190

191191
const auto start{SteadyClock::now()};
192192
const auto path_addr{args.GetDataDirNet() / "peers.dat"};
@@ -200,19 +200,17 @@ std::optional<bilingual_str> LoadAddrman(const NetGroupManager& netgroupman, con
200200
DumpPeerAddresses(args, *addrman);
201201
} catch (const InvalidAddrManVersionError&) {
202202
if (!RenameOver(path_addr, (fs::path)path_addr + ".bak")) {
203-
addrman = nullptr;
204-
return strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again."));
203+
return util::Error{strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again."))};
205204
}
206205
// Addrman can be in an inconsistent state after failure, reset it
207206
addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);
208207
LogPrintf("Creating new peers.dat because the file version was not compatible (%s). Original backed up to peers.dat.bak\n", fs::quoted(fs::PathToString(path_addr)));
209208
DumpPeerAddresses(args, *addrman);
210209
} catch (const std::exception& e) {
211-
addrman = nullptr;
212-
return strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."),
213-
e.what(), PACKAGE_BUGREPORT, fs::quoted(fs::PathToString(path_addr)));
210+
return util::Error{strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."),
211+
e.what(), PACKAGE_BUGREPORT, fs::quoted(fs::PathToString(path_addr)))};
214212
}
215-
return std::nullopt;
213+
return std::move(addrman); // std::move should be unneccessary but is temporarily needed to work around clang bug (https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561270092)
216214
}
217215

218216
void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors)

‎src/addrdb.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <net_types.h> // For banmap_t
1010
#include <univalue.h>
1111
#include <util/fs.h>
12+
#include <util/result.h>
1213

1314
#include <optional>
1415
#include <vector>
@@ -49,7 +50,7 @@ class CBanDB
4950
};
5051

5152
/** Returns an error string on failure */
52-
std::optional<bilingual_str> LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args, std::unique_ptr<AddrMan>& addrman);
53+
util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args);
5354

5455
/**
5556
* Dump the anchor IP address database (anchors.dat)

‎src/bitcoin-chainstate.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ int main(int argc, char* argv[])
6060
// We can't use a goto here, but we can use an assert since none of the
6161
// things instantiated so far requires running the epilogue to be torn down
6262
// properly
63-
assert(!kernel::SanityChecks(kernel_context).has_value());
63+
assert(kernel::SanityChecks(kernel_context));
6464

6565
// Necessary for CheckInputScripts (eventually called by ProcessNewBlock),
6666
// which will try the script cache first and fall back to actually

‎src/init.cpp‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,15 +1023,17 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
10231023
.chainparams = chainparams,
10241024
.datadir = args.GetDataDirNet(),
10251025
};
1026-
if (const auto error{ApplyArgsManOptions(args, chainman_opts_dummy)}) {
1027-
return InitError(*error);
1026+
auto chainman_result{ApplyArgsManOptions(args, chainman_opts_dummy)};
1027+
if (!chainman_result) {
1028+
return InitError(util::ErrorString(chainman_result));
10281029
}
10291030
BlockManager::Options blockman_opts_dummy{
10301031
.chainparams = chainman_opts_dummy.chainparams,
10311032
.blocks_dir = args.GetBlocksDirPath(),
10321033
};
1033-
if (const auto error{ApplyArgsManOptions(args, blockman_opts_dummy)}) {
1034-
return InitError(*error);
1034+
auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)};
1035+
if (!blockman_result) {
1036+
return InitError(util::ErrorString(blockman_result));
10351037
}
10361038
}
10371039

@@ -1054,8 +1056,9 @@ static bool LockDataDirectory(bool probeOnly)
10541056
bool AppInitSanityChecks(const kernel::Context& kernel)
10551057
{
10561058
// ********************************************************* Step 4: sanity checks
1057-
if (auto error = kernel::SanityChecks(kernel)) {
1058-
InitError(*error);
1059+
auto result{kernel::SanityChecks(kernel)};
1060+
if (!result) {
1061+
InitError(util::ErrorString(result));
10591062
return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), PACKAGE_NAME));
10601063
}
10611064

@@ -1230,9 +1233,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
12301233
// Initialize addrman
12311234
assert(!node.addrman);
12321235
uiInterface.InitMessage(_("Loading P2P addresses…").translated);
1233-
if (const auto error{LoadAddrman(*node.netgroupman, args, node.addrman)}) {
1234-
return InitError(*error);
1235-
}
1236+
auto addrman{LoadAddrman(*node.netgroupman, args)};
1237+
if (!addrman) return InitError(util::ErrorString(addrman));
1238+
node.addrman = std::move(*addrman);
12361239
}
12371240

12381241
assert(!node.banman);
@@ -1434,13 +1437,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14341437
.datadir = args.GetDataDirNet(),
14351438
.adjusted_time_callback = GetAdjustedTime,
14361439
};
1437-
Assert(!ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction
1440+
Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction
14381441

14391442
BlockManager::Options blockman_opts{
14401443
.chainparams = chainman_opts.chainparams,
14411444
.blocks_dir = args.GetBlocksDirPath(),
14421445
};
1443-
Assert(!ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
1446+
Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
14441447

14451448
// cache size calculations
14461449
CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size());
@@ -1463,8 +1466,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14631466
.estimator = node.fee_estimator.get(),
14641467
.check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0,
14651468
};
1466-
if (const auto err{ApplyArgsManOptions(args, chainparams, mempool_opts)}) {
1467-
return InitError(*err);
1469+
auto result{ApplyArgsManOptions(args, chainparams, mempool_opts)};
1470+
if (!result) {
1471+
return InitError(util::ErrorString(result));
14681472
}
14691473
mempool_opts.check_ratio = std::clamp<int>(mempool_opts.check_ratio, 0, 1'000'000);
14701474

@@ -1563,8 +1567,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
15631567
// If reindex-chainstate was specified, delay syncing indexes until ThreadImport has reindexed the chain
15641568
if (!fReindexChainState) g_indexes_ready_to_sync = true;
15651569
if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
1566-
if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}) {
1567-
return InitError(*error);
1570+
auto result{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))};
1571+
if (!result) {
1572+
return InitError(util::ErrorString(result));
15681573
}
15691574

15701575
g_txindex = std::make_unique<TxIndex>(interfaces::MakeChain(node), cache_sizes.tx_index, false, fReindex);

‎src/kernel/checks.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,21 @@
1313

1414
namespace kernel {
1515

16-
std::optional<bilingual_str> SanityChecks(const Context&)
16+
util::Result<void> SanityChecks(const Context&)
1717
{
1818
if (!ECC_InitSanityCheck()) {
19-
return Untranslated("Elliptic curve cryptography sanity check failure. Aborting.");
19+
return util::Error{Untranslated("Elliptic curve cryptography sanity check failure. Aborting.")};
2020
}
2121

2222
if (!Random_SanityCheck()) {
23-
return Untranslated("OS cryptographic RNG sanity check failure. Aborting.");
23+
return util::Error{Untranslated("OS cryptographic RNG sanity check failure. Aborting.")};
2424
}
2525

2626
if (!ChronoSanityCheck()) {
27-
return Untranslated("Clock epoch mismatch. Aborting.");
27+
return util::Error{Untranslated("Clock epoch mismatch. Aborting.")};
2828
}
2929

30-
return std::nullopt;
30+
return {};
3131
}
3232

3333
}

‎src/kernel/checks.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#ifndef BITCOIN_KERNEL_CHECKS_H
66
#define BITCOIN_KERNEL_CHECKS_H
77

8-
#include <optional>
8+
#include <util/result.h>
99

1010
struct bilingual_str;
1111

@@ -16,7 +16,7 @@ struct Context;
1616
/**
1717
* Ensure a usable environment with all necessary library support.
1818
*/
19-
std::optional<bilingual_str> SanityChecks(const Context&);
19+
util::Result<void> SanityChecks(const Context&);
2020

2121
}
2222

‎src/node/blockmanager_args.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,33 @@
77
#include <common/args.h>
88
#include <node/blockstorage.h>
99
#include <tinyformat.h>
10+
#include <util/result.h>
1011
#include <util/translation.h>
1112
#include <validation.h>
1213

1314
#include <cstdint>
14-
#include <optional>
1515

1616
namespace node {
17-
std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts)
17+
util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts)
1818
{
1919
// block pruning; get the amount of disk space (in MiB) to allot for block & undo files
2020
int64_t nPruneArg{args.GetIntArg("-prune", opts.prune_target)};
2121
if (nPruneArg < 0) {
22-
return _("Prune cannot be configured with a negative value.");
22+
return util::Error{_("Prune cannot be configured with a negative value.")};
2323
}
2424
uint64_t nPruneTarget{uint64_t(nPruneArg) * 1024 * 1024};
2525
if (nPruneArg == 1) { // manual pruning: -prune=1
2626
nPruneTarget = BlockManager::PRUNE_TARGET_MANUAL;
2727
} else if (nPruneTarget) {
2828
if (nPruneTarget < MIN_DISK_SPACE_FOR_BLOCK_FILES) {
29-
return strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024);
29+
return util::Error{strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024)};
3030
}
3131
}
3232
opts.prune_target = nPruneTarget;
3333

3434
if (auto value{args.GetBoolArg("-fastprune")}) opts.fast_prune = *value;
3535
if (auto value{args.GetBoolArg("-stopafterblockimport")}) opts.stop_after_block_import = *value;
3636

37-
return std::nullopt;
37+
return {};
3838
}
3939
} // namespace node

‎src/node/blockmanager_args.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@
77
#define BITCOIN_NODE_BLOCKMANAGER_ARGS_H
88

99
#include <node/blockstorage.h>
10-
11-
#include <optional>
10+
#include <util/result.h>
1211

1312
class ArgsManager;
14-
struct bilingual_str;
1513

1614
namespace node {
17-
std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts);
15+
util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts);
1816
} // namespace node
1917

2018
#endif // BITCOIN_NODE_BLOCKMANAGER_ARGS_H

‎src/node/chainstatemanager_args.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,24 @@
1111
#include <node/database_args.h>
1212
#include <tinyformat.h>
1313
#include <uint256.h>
14+
#include <util/result.h>
1415
#include <util/strencodings.h>
1516
#include <util/translation.h>
1617
#include <validation.h>
1718

1819
#include <chrono>
19-
#include <optional>
2020
#include <string>
2121

2222
namespace node {
23-
std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts)
23+
util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts)
2424
{
2525
if (auto value{args.GetBoolArg("-checkblockindex")}) opts.check_block_index = *value;
2626

2727
if (auto value{args.GetBoolArg("-checkpoints")}) opts.checkpoints_enabled = *value;
2828

2929
if (auto value{args.GetArg("-minimumchainwork")}) {
3030
if (!IsHexNumber(*value)) {
31-
return strprintf(Untranslated("Invalid non-hex (%s) minimum chain work value specified"), *value);
31+
return util::Error{strprintf(Untranslated("Invalid non-hex (%s) minimum chain work value specified"), *value)};
3232
}
3333
opts.minimum_chain_work = UintToArith256(uint256S(*value));
3434
}
@@ -41,6 +41,6 @@ std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, Chains
4141
ReadDatabaseArgs(args, opts.coins_db);
4242
ReadCoinsViewArgs(args, opts.coins_view);
4343

44-
return std::nullopt;
44+
return {};
4545
}
4646
} // namespace node

‎src/node/chainstatemanager_args.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,13 @@
55
#ifndef BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H
66
#define BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H
77

8+
#include <util/result.h>
89
#include <validation.h>
910

10-
#include <optional>
11-
1211
class ArgsManager;
13-
struct bilingual_str;
1412

1513
namespace node {
16-
std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts);
14+
util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts);
1715
} // namespace node
1816

1917
#endif // BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H

0 commit comments

Comments
 (0)