This PR is part of the process separation project.
This change is move-only and can be easily reviewed with --color-moved=dimmed_zebra. The moves are needed to avoid duplicating common init code between different binaries (bitcoin-node, bitcoin-wallet, etc) in #10102. In #10102, each binary has it’s own init file (src/init/bitcoin-node.cpp, src/init/bitcoin-wallet.cpp) so this PR moves the common code to src/init/common.cpp.
pr/initc.1 -> pr/initc.2, compare) to fix hidden_args CI errors https://cirrus-ci.com/task/4601873798791168?logs=ci#L3751 https://cirrus-ci.com/task/5446298728923136?logs=ci#L2663 https://cirrus-ci.com/task/5516667473100800?logs=ci#L4299 https://cirrus-ci.com/task/6079617426522112?logs=ci#L2817 https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38776308
Code review ACK 66bbbc09e9bb44546ac47829384cd3b088c5efd7.
Verified commit by commit, with --color-moved=dimmed-zebra. Moved code is tidy up in nice functions. I don’t observe changes in behavior.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
Concept ACK
Nice with the init namespace too: it makes it easier to reason about encapsulation and more specifically it allows for finding all non-init call sites just by doing git grep "init::".
More generally I think it would make sense to gradually introduce more of this type of namespace use in our project.
0@@ -0,0 +1,30 @@
1+// Copyright (c) 2021 The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+#include <key.h>
6+#include <logging.h>
7+#include <pubkey.h>
8+#include <random.h>
#include <crypto/sha256.h> be here too or not (for SHA256AutoDetect)?
re: #21732 (review)
Should
#include <crypto/sha256.h>be here too or not (forSHA256AutoDetect)?
Thanks, added
13+
14+namespace init {
15+void SetGlobals()
16+{
17+ // Initialize elliptic curve code
18+ std::string sha256_algo = SHA256AutoDetect();
1124@@ -1149,7 +1125,7 @@ bool AppInitSanityChecks()
1125 init::SetGlobals();
1126
1127 // Sanity check
1128- if (!InitSanityCheck())
1129+ if (!init::SanityChecks())
{ while touching this line?
1124@@ -1149,7 +1125,7 @@ bool AppInitSanityChecks()
1125 init::SetGlobals();
1126
1127 // Sanity check
re: #21732 (review)
e01dfce: Could remove comment that just repeats the function name that is called?
Thanks, removed
15+void UnsetGlobals();
16+/** Sanity checks
17+ * Ensure that Bitcoin is running in a usable environment with all
18+ * necessary library support.
19+ */
20+bool SanityChecks();
e01dfceb3c2f355e8b9979eb6aa6c41550c37787: Could remove comment that just repeats the function name?
0/**
1 * Ensure a usable environment with all
2 * necessary library support.
3 */
4bool SanityChecks();
re: #21732 (review)
e01dfce: Could remove comment that just repeats the function name?
Thanks, used suggestion
21@@ -22,6 +22,7 @@
22 #include <httpserver.h>
23 #include <index/blockfilterindex.h>
24 #include <index/txindex.h>
25+#include <init/common.h>
26 #include <interfaces/chain.h>
27 #include <interfaces/node.h>
28 #include <key.h>
re: #21732 (review)
000e94b: Could remove pubkey/key include, now that they are no longer used?
Thanks, removed
re: #21732 (review)
pubkey not addressed?
Yes I dropped <key.h> but didn’t see any <pubkey.h> so assumed it was mistake. Are we looking at the same file https://github.com/ryanofsky/bitcoin/blob/pr/initc.3/src/init.cpp?
Left some unrelated doc feedback to remove nonsense comments like
0// Sanity checks
1SanityChecks();
–color-moved=dimmed-zebra ACK 66bbbc09e9bb44546ac47829384cd3b088c5efd7 😹
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3- --color-moved=dimmed-zebra ACK 66bbbc09e9bb44546ac47829384cd3b088c5efd7 😹
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUjyPQwAltkH6kZvb6r7SNqiODJqjD0Ta96ZqC0rKjnSwMrLmObSuMu8OgWJ1zJW
8c2q68lGd1PGZHvDiIrlJu6939yGby5c+hj84iCkgJoL720ZrkXVQynPjtkzcOcEc
9vfa5VGH18547lkFyih1LYepSJ9MBnU+XH4WYZJMyeu75NVEc3OZTCj3xG1U519n4
10NyAdTJ8WK9KLZF8rYRP0/sBgXBawZtjaS43YSrdz061Ht8kTlU/YqQinbRs4mm0K
11EdRybSUXzDeFxGHCq2Kz6vuooSDasiTZUoVf66crNDN5XEIpmI2NgBDt8SjssQIg
12RQQ9vNGoHDqM4b0lI+zWc9kkply/82ZL1TJP1HBYy/gdVh+Z5M2L3q9RVOt315EJ
13VZnfl2fp2NNJoqcP/mLx2ZF4eZ5hNWJLiFw5bdDvmnbb9Gz3rS4lWg+0MrW3P6Bs
14XSb+eDLPgbluTlH5Azkn/j2KtuZf5ODk57igT6hg4ZxVwrspkoXIFckgitA3ddXu
15eN/gtak+
16=4TTx
17-----END PGP SIGNATURE-----
Timestamp of file with hash 27c6f524949cc5fa807c0a855609234ba5afe961ff88e1b28074374eea19342f -
pr/initc.2 -> pr/initc.3, compare) with suggested changes
review ACK 615965cfd1ef1e0627d69970d99bdfedb9176833 🖱
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3review ACK 615965cfd1ef1e0627d69970d99bdfedb9176833 🖱
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUj7CQv/Uj0PueOUes2yUiwCDP3lWhYevwJ0p5jaA85RtfTwsWnIAClUV0UmHAJh
8cNpmWT73Dgg8Ku0jQbHO0cf9adIeKv8sbA3f+jQeStiz8FogOR6CWtgX4RzbLAWZ
9W+x2Dq2ReHgToHggkQLWbEATGcZr2cQtub5NSjEwyfHnjIf8OvI7VOV7Ga8tsAWk
103oHSdjnWkfOcJLAkUZD9SF40fTjjPWFzI4KTwvp8SrQhL+uvuA4/3nk74idlCYE9
11tYmWuiJ5MFocDOF5MVV/OtmWt+vfI7w2wbRYGu0N7LQU5lB9y2UgB8RzTmOCpKDz
12qIrjsPdBNTiFcMOSYpJTE0s+Efuixi53nbHCWaJX2mgu7yzHZs/ktxw8NGeTUU2u
135nNs147u2YUaQ//aNcdbvRc7etvCT+do3N4zAYvNw9GNSHTjjPWT4uStX1qXMNnd
14UqbUfMUGUTDjqgIcG8bpPX6WegkUIrCi7GZwbCLVf0FjXvDd4rWkxfS4Ijpy9rh/
15/CfJrvyw
16=XCNq
17-----END PGP SIGNATURE-----
Timestamp of file with hash 460d9d89b06872cded3ec1cd1b35c0c9337114cc3b7071cebe084cad1768397b -