refactor: Move chain constants to the util library

TheCharlatan commented at 10:19 am on April 19, 2023: contributor

This pull request is part of the libbitcoinkernel project #24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its “Step 2: Decouple most non-consensus code from libbitcoinkernel”. It is also a follow up to #26177.

It replaces pull request #27294, which just moved the constants to a new file, but did not re-declare them as enums.

The code move of the chain name constants out of the chainparamsbase to their own separate header allows the kernel chainparams to no longer include chainparamsbase. The chainparamsbase contain references to the ArgsManager and networking related options that should not belong to the kernel library. Besides this move, the constants are re-declared as enums with helper functions facilitating string conversions.

DrahtBot commented at 10:19 am on April 19, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27596 (assumeutxo (2) by jamesob)
#27534 (rpc: add ‘getnetmsgstats’, new rpc to view network message statistics by satsie)
#27125 (refactor, kernel: Decouple ArgsManager from blockstorage by TheCharlatan)
#27039 (blockstorage: do not flush block to disk if it is already there by pinheadmz)
#26564 (test: allow BITCOIN_TEST_PATH to specify working dir by LarryRuane)
#26403 ([POLICY] Ephemeral anchors by instagibbs)
#25977 (refactor: Replace std::optional<bilingual_str> with util::Result by ryanofsky)
#25908 (p2p: remove adjusted time by fanquake)
#25038 (policy: nVersion=3 and Package RBF by glozow)
#17783 (util: Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky)
#17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
#17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
#17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)
#16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

TheCharlatan renamed this:
~~Kernel chain type~~
refactor: Move chain constants to the util library
on Apr 19, 2023

DrahtBot added the label Refactoring on Apr 19, 2023

fanquake commented at 10:22 am on April 19, 2023: member

07ea09dd60  scripted-diff: Remove unused chainparamsbase includes
1d99bd93e0 Add missing definitions in prep for scripted diff
25589a4b0d refactor: Replace string chain name constants with ChainTypes
3bd6493bf3 refactor: Create chaintype files
4
5Error: script block marker but no scripted-diff in title of commit 7ea09dd60cd43e943f9910c950b05dda89ef3725
6Failed

TheCharlatan force-pushed on Apr 19, 2023

TheCharlatan commented at 10:31 am on April 19, 2023: contributor

Re #27491 (comment)

7ea09dd60 scripted-diff: Remove unused chainparamsbase includes

Seems like the trailing whitespace in the commit mesasge header was tripping it up?

in src/chainparams.cpp:100 in 5589a4b0dd outdated

 96@@ -96,26 +97,30 @@ const CChainParams &Params() {
 97     return *globalChainParams;
 98 }
 99 
100-std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const std::string& chain)
101+std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const ChainType& chain)

ryanofsky commented at 11:00 am on April 19, 2023:

In commit “refactor: Replace string chain name constants with ChainTypes” (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

Maybe replace const ChainType& with ChainType everywhere since it’s an enum and probably more efficient to pass by value instead of by reference

in src/util/chaintype.cpp:23 in bd6493bf37 outdated

18+    case ChainType::SIGNET:
19+        return "signet";
20+    case ChainType::REGTEST:
21+        return "regtest";
22+    default:
23+        throw std::invalid_argument("Invalid ChainType value");

ryanofsky commented at 4:39 pm on April 19, 2023:

In commit “refactor: Create chaintype files” (bd6493bf37554b1fc5848d81bce2bc5487a952dd)

Might be better to move the throw after the switch statement and drop the default: case so the compiler would warn about unhandled enum values

Same suggestion applies to CreateChainParams and CreateBaseChainParams in the next commit

in src/qt/guiutil.cpp:509 in 5589a4b0dd outdated

507-    if (chain == CBaseChainParams::MAIN)
508+    ChainType chain = gArgs.GetChainType();
509+    if (chain == ChainType::MAIN)
510         return GetSpecialFolderPath(CSIDL_STARTUP) / "Bitcoin.lnk";
511-    if (chain == CBaseChainParams::TESTNET) // Remove this special case when CBaseChainParams::TESTNET = "testnet4"
512+    if (chain == ChainType::TESTNET) // Remove this special case when ChainType::TESTNET = "testnet4"

ryanofsky commented at 4:58 pm on April 19, 2023:

In commit “refactor: Replace string chain name constants with ChainTypes” (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

This comment is a little confusing, would suggest // Remove this special case when testnet CBaseChainParams::DataDir() is incremented to "testnet4"

in src/util/system.cpp:933 in 5589a4b0dd outdated

928@@ -930,7 +929,17 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
929     return true;
930 }
931 
932-std::string ArgsManager::GetChainName() const

ryanofsky commented at 5:12 pm on April 19, 2023:

In commit “refactor: Replace string chain name constants with ChainTypes” (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

I noticed there are still mentions some of the GetChainName functions in comments that could be replaced (git grep -n GetChainName)

in src/util/system.cpp:934 in 5589a4b0dd outdated

928@@ -930,7 +929,17 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
929     return true;
930 }
931 
932-std::string ArgsManager::GetChainName() const
933+ChainType ArgsManager::GetChainType() const
934+{
935+    std::string chain_str = GetChainTypeString();

ryanofsky commented at 5:52 pm on April 19, 2023:

In commit “refactor: Replace string chain name constants with ChainTypes” (5589a4b0dd5b25017299f666127b7537f5f7a4c1)

It seems messy and potentially error prone for these functions convert from enum values to strings and back to enums again. Would suggest avoiding unneeded conversions with something like:

 0std::variant<ChainType, std::string> ArgsManager::GetChainArg() const
 1{
 2    ...
 3    const auto chain_arg = GetArg("-chain");
 4    const bool fRegTest = get_net("-regtest");
 5    const bool fSigNet  = get_net("-signet");
 6    const bool fTestNet = get_net("-testnet");
 7
 8    if ((int)chain_arg.has_value() + (int)fRegTest + (int)fSigNet + (int)fTestNet > 1) {
 9        throw std::runtime_error("Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.");
10    }
11    if (chain_arg) {
12        if (auto parsed = ChainTypeFromString(*chain_arg)) return *parsed;
13        // Not a known type string, so return original string.
14        return *chain_arg;
15    }
16    if (fRegTest) return ChainType::REGTEST;
17    if (fSigNet) return ChainType::SIGNET;
18    if (fTestNet) return ChainType::TESTNET;
19    return ChainType::MAIN;
20}
21
22ChainType ArgsManager::GetChainType() const
23{
24    auto arg = GetChainArg();
25    if (auto* parsed = std::get_if<ChainType>(&arg)) return *parsed;
26    throw std::runtime_error(strprintf("Unknown chain %s.", std::get<std::string>(arg)));
27}
28
29std::string ArgsManager::GetChainTypeString() const
30{
31    auto arg = GetChainArg();
32    if (auto* parsed = std::get_if<ChainType>(&arg)) return ChainTypeToString(*parsed);
33    return std::get<std::string>(arg);
34}

ryanofsky approved

ryanofsky commented at 6:04 pm on April 19, 2023: contributor

Code review ACK bbe05915adad45560907b5a9b58fc55052838c08. Left a few suggestions, but this looks good in its current form

in src/util/chaintype.h:14 in bd6493bf37 outdated

 9+#include <string>
10+
11+enum class ChainType { MAIN,
12+                       TESTNET,
13+                       SIGNET,
14+                       REGTEST };

ajtowns commented at 4:47 am on April 20, 2023:

Put MAIN on a newline and just use 4-space indentation? Newlines for { and } as well, per style guide? Trailing , after REGTEST in case we add any new ones later?

TheCharlatan commented at 8:59 pm on April 20, 2023:

The style guide is vague on enums (though I guess you could follow the recommendation for classes)

0  - Braces on new lines for classes, functions, methods.
1  - Braces on the same line for everything else.

and our code (even the most recent additions) has a mix between putting braces on the same line and a new line for enums.

in src/util/chaintype.cpp:11 in bd6493bf37 outdated

 6+
 7+#include <optional>
 8+#include <stdexcept>
 9+#include <string>
10+
11+std::string ChainTypeToString(ChainType chain)

ajtowns commented at 4:58 am on April 20, 2023:

Calling it ChainName(ChainType chain) might be a bit clearer?

TheCharlatan commented at 8:59 pm on April 20, 2023:

I’m not sure on this one. Granted, std::string ChainName(ChainType chain) would make it clear for the caller that this is most likely a label associated with the ChainType. However, for consistency I would also have to rename the helper functions to GetChainName(), and that seems less clear, because it does not have a typed argument allowing the caller to infer the relationship with ChainType.

ajtowns commented at 5:00 am on April 20, 2023: contributor

Would be better if the second commit (renaming CBaseChainParam::MAIN to ChainType::MAIN etc) could be a scripted-diff. Perhaps structure it as:

introduce ChainType
add CreateBaseChainParams(ChainType) and replace CreateBaseChainParams(string) with { auto ct = ChainTypeFromString(chain); if (ct) { return CreateBaseChainParams(ct); } else { throw...; } (and so on)
scripted-diff to switch to ChainType::MAIN and args.GetChainType()
remove the now unused functions that accepted a string

TheCharlatan commented at 5:01 pm on April 20, 2023: contributor

scripted-diff to switch to ChainType::MAIN and args.GetChainType()

I don’t think this is feasible, because we have to decide on the type (either calling a helper function for a string or the enum) based on the context, no?

TheCharlatan force-pushed on Apr 20, 2023

TheCharlatan commented at 8:58 pm on April 20, 2023: contributor

Thank you for the reviews!

Updated bbe05915adad45560907b5a9b58fc55052838c08 -> 32a70863215ac14b6402fd2a054aedbfe01ff30d (kernelChainType_0 -> kernelChainType_1, compare)

Addressed @ryanofsky’s comment, removing ChainType pass by reference.
Addressed @ryanofsky’s comment, removing the switch statement default cases.
Addressed @ryanofsky’s comment, updating the comment as suggested.
Addressed @ryanofsky’s comment, cleaning up remaining mentions of the now renamed functions and variable names
Addressed @ryanofsky’s comment, adding a commit with the suggested refactor for avoiding needless type conversions.
Addressed @ajtown’s comment, improving enum declaration formatting.

DrahtBot added the label Needs rebase on Apr 21, 2023

TheCharlatan force-pushed on Apr 21, 2023

TheCharlatan commented at 2:06 pm on April 21, 2023: contributor

Rebased 32a70863215ac14b6402fd2a054aedbfe01ff30d -> 7d361083d72a267de4af258e574219abdef0fc82 (kernelChainType_1 -> kernelChainType_2, compare)

DrahtBot removed the label Needs rebase on Apr 21, 2023

DrahtBot added the label Needs rebase on May 5, 2023

TheCharlatan force-pushed on May 5, 2023

TheCharlatan commented at 8:31 pm on May 5, 2023: contributor

Rebased 7d361083d72a267de4af258e574219abdef0fc82 -> dd95e2a3353b5ded87d1d5408a51bf461f1f90b4 (kernelChainType_2 -> kernelChainType_3, compare)

Fixed conflict with #27570

DrahtBot removed the label Needs rebase on May 5, 2023

DrahtBot added the label Needs rebase on May 6, 2023

TheCharlatan force-pushed on May 6, 2023

TheCharlatan commented at 5:02 pm on May 6, 2023: contributor

Rebased dd95e2a3353b5ded87d1d5408a51bf461f1f90b4 -> 95744f9cf1f143b6449a5046a914557b3886e3a2 (kernelChainType_3 -> kernelChainType_4, compare)

Fixed conflict with #17860

DrahtBot removed the label Needs rebase on May 6, 2023

in src/bitcoin-cli.cpp:432 in 95744f9cf1 outdated

426@@ -426,9 +427,9 @@ class NetinfoRequestHandler : public BaseRequestHandler
427     std::vector<Peer> m_peers;
428     std::string ChainToString() const
429     {
430-        if (gArgs.GetChainName() == CBaseChainParams::TESTNET) return " testnet";
431-        if (gArgs.GetChainName() == CBaseChainParams::SIGNET) return " signet";
432-        if (gArgs.GetChainName() == CBaseChainParams::REGTEST) return " regtest";
433+        if (gArgs.GetChainType() == ChainType::TESTNET) return " testnet";
434+        if (gArgs.GetChainType() == ChainType::SIGNET) return " signet";
435+        if (gArgs.GetChainType() == ChainType::REGTEST) return " regtest";

maflcko commented at 1:22 pm on May 8, 2023:

Shouldn’t use this switch-case, now that the type is an enum class?

maflcko commented at 4:21 pm on May 9, 2023:

Well, I mean that the default case can now be skipped. See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures section enum class - default:

maflcko commented at 4:21 pm on May 9, 2023:

(Otherwise, if a new chain type is added, it will silently map to main)

in src/util/chaintype.cpp:23 in 95744f9cf1 outdated

18+    case ChainType::SIGNET:
19+        return "signet";
20+    case ChainType::REGTEST:
21+        return "regtest";
22+    }
23+    throw std::invalid_argument("Invalid ChainType value");

maflcko commented at 1:30 pm on May 8, 2023:

I think unreachable code can be Assert(false)?

TheCharlatan commented at 8:54 pm on May 8, 2023:

Adding Assert(false) seems to throw an error:

0util/chaintype.cpp: In function ‘std::string ChainTypeToString(ChainType)’:
1./util/check.h:73:89: error: control reaches end of non-void function [-Werror=return-type]
2   73 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
3      |                                                                                         ^
4util/chaintype.cpp:23:5: note: in expansion of macro ‘Assert’
5   23 |     Assert(false);
6      |     ^~~~~~

maflcko commented at 7:28 am on May 9, 2023:

ah ok, maybe assert(false)

TheCharlatan commented at 10:24 am on May 9, 2023:

That works :)

in src/common/args.cpp:722 in ca13faf0cc outdated

717@@ -717,14 +718,24 @@ fs::path ArgsManager::GetConfigFilePath() const
718     return GetConfigFile(*this, GetPathArg("-conf", BITCOIN_CONF_FILENAME));
719 }
720 
721-std::string ArgsManager::GetChainName() const
722+ChainType ArgsManager::GetChainType() const

ryanofsky commented at 2:05 pm on May 8, 2023:

In commit “refactor: Replace string chain name constants with ChainTypes” (ca13faf0cc8acbaf425ac69c783415465403b5a3)

Could this new method be added in an earlier commit? This commit is very long and all the other changes in the commit are mechanical changes except this one method, so the new code seems to get lost.

(This would also open the door to automating the other changes in this commit with a scripted-diff, though that would still be awkward not and not necessary IMO)

in src/common/args.h:430 in 95744f9cf1 outdated

420@@ -420,6 +421,8 @@ class ArgsManager
421      */
422     const fs::path& GetDataDir(bool net_specific) const;
423 
424+    std::variant<ChainType, std::string> GetChainArg() const;

ryanofsky commented at 2:17 pm on May 8, 2023:

In commit “refactor: Avoid needless chain type string conversions” (95744f9cf1f143b6449a5046a914557b3886e3a2)

Would be helpful to have a code comment describing what this does. Like “Return -regtest/-signet/-testnet/-chain= setting as a ChainType enum if a recognized chain name was set, or as a string if an unrecognized chain name was set. Raise an exception if an invalid combination of flags was provided.

in src/common/args.cpp:736 in 95744f9cf1 outdated

738+    auto arg = GetChainArg();
739+    if (auto* parsed = std::get_if<ChainType>(&arg)) return ChainTypeToString(*parsed);
740+    return std::get<std::string>(arg);
741+}
742+
743+std::variant<ChainType, std::string> ArgsManager::GetChainArg() const

ryanofsky commented at 2:25 pm on May 8, 2023:

In commit “refactor: Avoid needless chain type string conversions” (95744f9cf1f143b6449a5046a914557b3886e3a2)

It seems like this commit could be moved before the “refactor: Replace string chain name constants with ChainTypes” (ca13faf0cc8acbaf425ac69c783415465403b5a3)" commit, which would simplify that commit, and avoid for the ArgsManager::GetChainType() method to change after it is added. I think the only change you would need to make to the code in this commit to move it earlier would be to keep the ArgsManager::GetChainTypeString() method called ArgsManager::GetChainName()

ryanofsky approved

ryanofsky commented at 2:31 pm on May 8, 2023: contributor

Code review ACK 95744f9cf1f143b6449a5046a914557b3886e3a2.

I think this looks good in it’s current form, but the PR would have less churn and be simpler overall if the last commit “refactor: Avoid needless chain type string conversions” (95744f9cf1f143b6449a5046a914557b3886e3a2) was done earlier and became the second commit before commit “refactor: Replace string chain name constants with ChainTypes” (ca13faf0cc8acbaf425ac69c783415465403b5a3)

TheCharlatan force-pushed on May 8, 2023

TheCharlatan commented at 9:44 pm on May 8, 2023: contributor

Thank you for the reviews!

Updated 95744f9cf1f143b6449a5046a914557b3886e3a2 -> 43dec8806f679005d4484b229f433687526c9133 (kernelChainType_4 -> kernelChainType_5, compare)

Addressed @MarcoFalke’s comment, adding another switch case
Addressed @ryanofsky’s comment_1, comment_2, re-ordering the commits and declaring the new ChainType getters before using them in the following commits
Addressed @ryanofsky’s comment, adding the proposed comment to GetChainArg in the common/args.h header
Improved GetChainType docstring in the common/args.hheader
Removed unneeded tuple include in common/args.cpp

refactor: Create chaintype files

This is the first of a number of commits with the goal of moving the
chain type definitions out of chainparamsbase to their own file and
implementing them as enums instead of constant strings. The goal is to
allow the kernel chainparams to no longer include chainparamsbase.

The commit is part of an ongoing effort to decouple the libbitcoinkernel
library from the ArgsManager and other functionality that should not be
part of the kernel library.

bfc21c31b2

TheCharlatan force-pushed on May 9, 2023

TheCharlatan commented at 10:24 am on May 9, 2023: contributor

Updated 43dec8806f679005d4484b229f433687526c9133 -> 1ae4d9a37b46a5e69ed79066408a6002c2cc0f73 (kernelChainType_5 -> kernelChainType_6, compare)

Addressed @MarcoFalke’s comment, changed throwing an exception to assert(false).

ryanofsky approved

ryanofsky commented at 1:19 pm on May 9, 2023: contributor

Code review ACK 1ae4d9a37b46a5e69ed79066408a6002c2cc0f73, but I would suggest another change to make second and third commits more reviewable (see below).

re: #27491 (comment)

re-ordering the commits and declaring the new ChainType getters before using them in the following commits

Thanks for the update, but this sort of does the opposite of what I was trying to suggest. I’m trying move substantive code changes out of the giant 3rd commit “Replace string chain name constants with ChainTypes”, into smaller earlier commits that are more understandable. I think the giant commit should only consist of mechanical replacements and not contain new code.

Here’s a branch with what I was suggesting: baa45381e64261c308e90071d8f3d85cdedb4b3a (branch). The final change is identical to what you have now, I just rearranged the second and third commits so both are smaller and mechanical changes aren’t mixed with substantive changes.

refactor: Introduce ChainType getters for ArgsManager

These are introduced for the next commit where the usage of the
ChainType is adopted throughout the code.

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: TheCharlatan <seb.kung@gmail.com>

401453df41

refactor: Replace string chain name constants with ChainTypes

This commit effectively moves the definition of these constants
out of the chainparamsbase to their own file.

Using the ChainType enums provides better type safety compared to
passing around strings.

The commit is part of an ongoing effort to decouple the libbitcoinkernel
library from the ArgsManager and other functionality that should not be
part of the kernel library.

ba8fc7d788

Add missing definitions in prep for scripted diff

The missing include and ArgsManager were found after applying the
scripted diff in the following commit.

e9ee8aaf3a

scripted-diff: Remove unused chainparamsbase includes

This is a follow-up to previous commits moving the chain constants out
of chainparamsbase.

The script removes the chainparamsbase header in all files where it is
included, but not used. This is done by filtering against all defined
symbols of the header as well as its respective .cpp file.

The kernel chainparams now no longer relies on chainparamsbase.

-BEGIN VERIFY SCRIPT-
sed -i '/#include <chainparamsbase.h>/d' $( git grep -l 'chainparamsbase.h' | xargs grep -L 'CBaseChainParams\|CreateBaseChainParams\|SetupChainParamsBaseOptions\|BaseParams\|SelectBaseParams\|chainparamsbase.cpp' )
-END VERIFY SCRIPT-

d168458d1f

TheCharlatan force-pushed on May 9, 2023

TheCharlatan commented at 2:06 pm on May 9, 2023: contributor

Thank you for your patience @ryanofsky

Updated 1ae4d9a37b46a5e69ed79066408a6002c2cc0f73 -> d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7 (kernelChainType_6 -> kernelChainType_7, compare)

Addressed @ryanofsky’s comment, changed commits as suggested. No net changes to the code, but should result in fewer touched lines in the history.

fanquake requested review from maflcko on May 9, 2023

fanquake requested review from ryanofsky on May 9, 2023

ryanofsky approved

ryanofsky commented at 2:17 pm on May 9, 2023: contributor

Code review ACK d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7. Just suggested changes since last review.

If you can, it would also be helpful to mark previous review comments as resolved (github won’t let me do this)

Thank you for your patience

Likewise! Thank you for all the improvements

fanquake commented at 2:29 pm on May 9, 2023: member

I’m going to merge this shortly. #27125 (which conflicts) will be rebased (some other conflicting changes may also be merged in the interim), along with its current set of review feedback addressed.

fanquake merged this on May 9, 2023

fanquake closed this on May 9, 2023

in src/chainparams.cpp:120 in d168458d1f

121         ReadRegTestArgs(args, opts);
122         return CChainParams::RegTest(opts);
123     }
124-    throw std::runtime_error(strprintf("%s: Unknown chain %s.", __func__, chain));
125+    }
126+    throw std::invalid_argument(strprintf("%s: Invalid ChainType value", __func__));

maflcko commented at 4:25 pm on May 9, 2023:

What is the point of adding dead code over assert(false)? Also, the docstring is now wrong. See also #27491 (review)

fanquake commented at 10:41 am on May 10, 2023:

Followed up in #27611.

in src/chainparamsbase.cpp:51 in d168458d1f

52-    } else if (chain == CBaseChainParams::REGTEST) {
53+    case ChainType::REGTEST:
54         return std::make_unique<CBaseChainParams>("regtest", 18443, 18445);
55     }
56-    throw std::runtime_error(strprintf("%s: Unknown chain %s.", __func__, chain));
57+    throw std::invalid_argument(strprintf("%s: Invalid ChainType value", __func__));

maflcko commented at 4:26 pm on May 9, 2023:

What is the point of adding dead code over assert(false)? Also, the docstring is now wrong. See also #27491 (review)

fanquake commented at 10:41 am on May 10, 2023:

Followed up in #27611.

maflcko commented at 4:50 pm on May 9, 2023: member

See also #14309 , where I added the ChainType enum class.

sidhujag referenced this in commit 8148f0ce5e on May 9, 2023

TheCharlatan referenced this in commit 1ee87074dc on May 9, 2023

TheCharlatan referenced this in commit d0767a9e0a on May 9, 2023

TheCharlatan referenced this in commit cf5744640a on May 10, 2023

TheCharlatan referenced this in commit 8783cb066a on May 10, 2023

TheCharlatan referenced this in commit e23088707b on May 10, 2023

in src/common/args.h:339 in d168458d1f

336+    /**
337+     * Returns the appropriate chain name string from the program arguments.
338+     * @return ChainType::MAIN string by default; raises runtime error if an
339+     * invalid combination is given.
340+     */
341+    std::string GetChainTypeString() const;

maflcko commented at 9:03 am on May 10, 2023:

I wonder if we can make those two getters return Results, instead of throwing exceptions.

I tried that in the past because there was an unguarded call in the gui, which would then cause a segfault, but that seems now fixed. Still, low-prio and long-term this may be a thing to keep in mind.

fanquake referenced this in commit 104eed1166 on May 10, 2023

sidhujag referenced this in commit bc01b73390 on May 10, 2023

0xB10C referenced this in commit 34f09f1a01 on May 15, 2023

RandyMcMillan referenced this in commit 7ff250908a on May 27, 2023

hebasto referenced this in commit 592da16150 on Aug 29, 2023

hebasto referenced this in commit c77e8b5c38 on Aug 29, 2023

hebasto referenced this in commit 8b025633e4 on Aug 29, 2023

hebasto referenced this in commit 14ddf61693 on Aug 31, 2023

janus referenced this in commit 896c32cb28 on Sep 11, 2023

bitcoin locked this on May 9, 2024

refactor: Move chain constants to the util library #27491

Reviews

Conflicts