aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-05-03rpc: Move output script RPCs to separate fileMacroFake
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-05-03Merge bitcoin/bitcoin#25029: rpc: Move fee estimation RPCs to separate fileMacroFake
fa753abd7cffa05548ad5f21f2e8f9f6b06a7b04 rpc: Move fee estimation RPCs to separate file (MacroFake) Pull request description: Fee estimation is generally used by wallets when creating txs. It doesn't have anything to do with creating or submitting blocks. ACKs for top commit: pk-b2: ACK https://github.com/bitcoin/bitcoin/pull/25029/commits/fa753abd7cffa05548ad5f21f2e8f9f6b06a7b04 brunoerg: crACK fa753abd7cffa05548ad5f21f2e8f9f6b06a7b04 Tree-SHA512: 81e0edc936198a0baf0f5bfa8cfedc12db51759c7873bb0082dfc5f0040d7f275b35f639c6f5b86fa1ea03397b0d5e757c2ce1b6b16f1029880a39b9c3aaceda
2022-05-02Merge bitcoin/bitcoin#25042: lint: Fix lint-circular-dependencies.py file listlaanwj
fad0abf539dc2141a3937f040e16703e38654fe3 lint: Fix lint-circular-dependencies.py file list (MacroFake) Pull request description: currently in-tree files like `wallet/test/fuzz/coinselection.cpp` are missed. Also out-of-tree files like `test/data/bip341_wallet_vectors.json.h` or `qt/moc_qvaluecombobox.cpp` are included. Change the script to only use in-tree files. Also, change `'python3'` to `sys.executable`. ACKs for top commit: laanwj: Code review ACK fad0abf539dc2141a3937f040e16703e38654fe3 Tree-SHA512: baf150fbae6a7120b2692f2eaef6a7773f2681e1610f8776f8d2ae6736c74736502a505df080b2182880f753b90f94e76a1e365fb45185f46f0e4d5521ca8e86
2022-05-02Merge bitcoin/bitcoin#25017: validation: make CScriptCheck and prevector ↵MacroFake
swap members noexcept e5485e8e4be7f2ee0671f58c3dcce35c68ba0ee0 test, bench: make prevector and checkqueue swap member functions noexcept (Jon Atack) abc1ee509025d92db5311c3f5df3b61c09cad24f validation: make CScriptCheck and prevector swap member functions noexcept (Jon Atack) Pull request description: along with those seen elsewhere in the codebase (prevector and checkqueue units/fuzz/bench). A swap must not fail; when a class has a swap member function, it should be declared noexcept. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c84-a-swap-function-must-not-fail ACKs for top commit: pk-b2: ACK https://github.com/bitcoin/bitcoin/pull/25017/commits/e5485e8e4be7f2ee0671f58c3dcce35c68ba0ee0 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25017/commits/e5485e8e4be7f2ee0671f58c3dcce35c68ba0ee0 Tree-SHA512: c82359d5e13f9262ce45efdae9baf71e41ed26568e0aff620e2bfb0ab37a62b6d56ae9340a28a0332c902cc1fa87da3fb72d6f6d6f53a8b7e695a5011f71f7f1
2022-05-01Merge bitcoin/bitcoin#25043: Reject invalid rpcauth formatsfanquake
fa12706fc6dbaf82eca37f30afa07c37fcd44932 Reject invalid rpcauth formats (MacroFake) Pull request description: This was added in commit 438ee59839ad49bf629452279478462c987b7137, but I couldn't determine if it was intentional. One reason to accept `foo:bar:baz` over `foo:bar$baz` is that `$` may be eaten by the shell. Though, I don't think many users pass `rpcauth` via the shell. Also it should be easy to avoid by passing `'-rpcauth=foo:bar$baz'` or `"-rpcauth=foo:bar\$baz"`. Can be tested with the added test. ACKs for top commit: pk-b2: ACK fa12706fc6dbaf82eca37f30afa07c37fcd44932 Tree-SHA512: 9998cbb295c79f7b0342bf86e1d3e5b5ab90851c627662ad6495b699a65a9035998173cf1debfd94325387faba184de683407b609fe86acdd8f6749157644441
2022-04-30Reject invalid rpcauth formatsMacroFake
2022-04-30Merge bitcoin/bitcoin#24543: net processing: Move remaining globals into ↵MacroFake
PeerManagerImpl 778343a379026ef233dffea67f5226565f6d5720 scripted-diff: Rename PeerManagerImpl members (dergoegge) 91c339243e11ec42eeeaca8fe015fc1c3e6338e1 [net processing] Move nHighestFastAnnounce into PeerManagerImpl (dergoegge) 10b83e2aa3393ef2c942fde7ac86e8cf3ea224c1 [net processing] Move block cache state into PeerManagerImpl (dergoegge) a4c55a93ef9277e1043c286120e2417652ee8bbb [net processing] Inline and simplify UpdatePreferredDownload (dergoegge) 490c08f96a34ed436c3d2cf7b9a3ed72694b6147 [net processing] Move nPreferredDownload into PeerManagerImpl (dergoegge) a292df283a596efe7e1d40c33a6d614d70ed564d [net processing] Move mapNodeState into PeerManagerImpl (dergoegge) 37ecaf3e7a028486a0a1c9b717e8eb4214215805 [net processing] Move CNodeState declaration above PeerManagerImpl (dergoegge) Pull request description: This PR moves the remaining net processing globals into `PeerManagerImpl`. This will make testing the peer manager in isolation easier and also acts as a code clean up. ACKs for top commit: jnewbery: Code review ACK 778343a379026ef233dffea67f5226565f6d5720 MarcoFalke: ACK 778343a379026ef233dffea67f5226565f6d5720 🗒 Tree-SHA512: 4f22105d1de37b94c3ef349f38784a30cf8d450d394a6a7849e5bd78940a71e3edbffa3d25e8efb35d7f698fd255f199de7bd4c33e23af5621a6e4e67ed43cb5
2022-04-30lint: Fix lint-circular-dependencies.py file listMacroFake
2022-04-30Merge bitcoin/bitcoin#25027: test: Remove boost::split from getarg_tests.cppfanquake
fafa7276126d21d7e2f723673b64382e16a902d9 test: Remove boost::split from getarg_tests.cpp (MacroFake) Pull request description: Only single spaces are used, so no need for boost. Can be tested with: ```diff diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index c877105fe7..a834830490 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -21,8 +21,11 @@ BOOST_FIXTURE_TEST_SUITE(getarg_tests, BasicTestingSetup) void ResetArgs(ArgsManager& local_args, const std::string& strArg) { std::vector<std::string> vecArg; - if (strArg.size()) + if (strArg.size()) { boost::split(vecArg, strArg, IsSpace, boost::token_compress_on); + auto vecArg2{SplitString(strArg, ' ')}; + assert(vecArg2 == vecArg); + } // Insert dummy executable name: vecArg.insert(vecArg.begin(), "testbitcoin"); ACKs for top commit: fanquake: utACK fafa7276126d21d7e2f723673b64382e16a902d9 - After this, the last usage of `<boost/algorithm/string.hpp>` is in `httprpc.cpp`. Tree-SHA512: 038af095cfb5240216305919cdeeb12d8e3ff0424520b99785bff5353a47dfcacdc049b927d7316b13e17a3c19b5f7549c9db7c4b5f2fa78ff1816515ca28d9d
2022-04-30Merge bitcoin/bitcoin#25028: ci: Clone iwyu only if missingfanquake
fa847ed2f698a8a999352aeed63baf60b094e662 ci: Clone iwyu only if missing (MacroFake) Pull request description: This doesn't change anything for Cirrus CI, but makes it easier to play locally. For reference, the same check is done when cloning `DIR_FUZZ_IN`. ACKs for top commit: fanquake: ACK fa847ed2f698a8a999352aeed63baf60b094e662 Tree-SHA512: 3d9689ea85b2380dcf83d26997c89c63f163aebfae9530180cb2420872a9f30d7b3dc59722e2e49684fdb3e30859b1e08e1272f6d083c07f213e9f5a190ca21f
2022-04-30Merge bitcoin/bitcoin#25034: test: add missing stop_node calls to ↵MacroFake
feature_coinstatsindex and feature_prune a3cd7dbfd8200c580aae9ea0f5473d58107dd582 test: stop node before calling assert_start_raises_init_error (Martin Zumsande) Pull request description: In #24789, I forgot to stop the node before using `assert_start_raises_init_error` in `feature_coinstatsindex`. This resulted in a bitcoind process that is not being terminated after the test finishes. `feature_prune` has the same problem and also creates a zombie bitcoind process. Also adds an assert to `assert_start_raises_init_error` to make sure the node isn't already running to prevent this sort of mistake in the future. Top commit has no ACKs. Tree-SHA512: 902f683ebe7b19ca32ab83ca40d9698e9d91509b1d003f21a7221f79b647e05b6ef5c0c888fbb772cbca5e641d5c9437d522b6671f446c3ab321d79f7c6d0284
2022-04-30ci: Clone iwyu only if missingMacroFake
2022-04-29test: stop node before calling assert_start_raises_init_errorMartin Zumsande
...in feature_coinstatsindex and feature_pruning. Also add an assert to assert_start_raises_init_error that the node is not already running.
2022-04-29rpc: Move fee estimation RPCs to separate fileMacroFake
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-04-29test: Remove boost::split from getarg_tests.cppMacroFake
2022-04-29Merge bitcoin/bitcoin#25025: test: Remove boost::split from rpc_tests.cppMacroFake
fad35e9afdd0bb6e8d6bf7f34a31de11aeb2d39b test: Remove boost::split from rpc_tests.cpp (MacroFake) Pull request description: No need for boost, as there are no tabs. Can be tested with: ```diff diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 50b5078110..ad6a888ad0 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -29,6 +29,7 @@ public: UniValue RPCTestingSetup::CallRPC(std::string args) { +Assert(args.find('\t')==std::string::npos); std::vector<std::string> vArgs; boost::split(vArgs, args, boost::is_any_of(" \t")); std::string strMethod = vArgs[0]; ACKs for top commit: fanquake: utACK fad35e9afdd0bb6e8d6bf7f34a31de11aeb2d39b Tree-SHA512: 3df789a222b407d61ad549adc4bbded00705d7c3db07472c31ce0e82216fe3ae27724b7f0ee3e85084bdf405cc28185e85487c9a7001620d6654fda77bab8eb3
2022-04-29Merge bitcoin/bitcoin#25016: refactor: GetFirstStoredBlock() and ↵fanquake
getblockchaininfo follow-ups e2b954e87f0c4cd5c8ac6e4d9c6b4d784844b4d2 rpc: use GetBlockTime() for getblockchaininfo#time (Jon Atack) 86ce844d3b287012f27c7b0bad6d11c9bdd3120e blockstorage, refactor: pass GetFirstStoredBlock() start_block by reference (Jon Atack) ed12c0a49d3c64d170aca9e66ef32a57d7933eeb blockstorage, refactor: make GetFirstStoredBlock() a member of BlockManager (Jon Atack) Pull request description: Picks up the remaining review feedback in #21726 and #24956. - make the global function `GetFirstStoredBlock()` a member of the `BlockManager` class - pass the `start_block` param of `GetFirstStoredBlock()` by reference instead of a pointer - use `GetBlockTime()` for RPC getblockchaininfo#time ACKs for top commit: MarcoFalke: ACK e2b954e87f0c4cd5c8ac6e4d9c6b4d784844b4d2 Tree-SHA512: 546e3c2e18245996b5b286829a605ae919eff3510963ec71b7c9ede521b1f501697e5b2f9d35d7a0606a74cbc8907201c58acf1e2cf7daaa86eefe2e3a8e296b
2022-04-29Merge bitcoin/bitcoin#25024: test: Split MempoolAncestryTests into twofanquake
fa2102e239a01fab648e60de915f9072c5544828 test: Split MempoolAncestryTests into two (MacroFake) Pull request description: The two tests don't share any state, so it seems clearer to put them in separate scopes. ACKs for top commit: jnewbery: Code review ACK fa2102e239 Tree-SHA512: 6669f50f8d5944fed55ecc88aa1bd139bddf6a40e3c2e8f88c3cc7e70cf6d4650c0dd652c7f304813893827c3930d626268655cd9b3f17ff9c9a1a02f0359714
2022-04-29Merge bitcoin/bitcoin#25013: Remove cs_main from verifymessage, move msg ↵fanquake
utils to new file fa60169811d6991a116bd37e1ff58049d2beee77 rpc: Move signmessage RPC util to new file (MacroFake) fa9425177e6be2c53fb5c1333636fa4d678ab401 Remove cs_main from verifymessage (MacroFake) Pull request description: The `verifymessage` RPC has several issues: * It takes `cs_main` for no reason, blocking progress on removing the `cs_main` global mutex. * It is located in a file called `misc`, which is not a very helpful name. Fix all issues. ACKs for top commit: vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25013/commits/fa60169811d6991a116bd37e1ff58049d2beee77 Tree-SHA512: c71a1f481b828e0a544405fecbbc7ca44e66ea46b498d7aed1f1c584d6a99724deb13e89d90b9d5cdeecbce293e6a41e9f7ae299543f6d761bf9e7a839b6c7f3
2022-04-29test: Remove boost::split from rpc_tests.cppMacroFake
2022-04-29test: Split MempoolAncestryTests into twoMacroFake
2022-04-29Merge bitcoin/bitcoin#25009: Crash debug builds on PCKG_MEMPOOL_ERRORMacroFake
fa10c9f5a1c9f8b37d51f43f98254feb9a8f9c53 Crash debug builds on PCKG_MEMPOOL_ERROR (MacroFake) Pull request description: Would be nice to allow fuzz targets to meaningfully cover this code ACKs for top commit: glozow: utACK fa10c9f5a1c9f8b37d51f43f98254feb9a8f9c53 vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25009/commits/fa10c9f5a1c9f8b37d51f43f98254feb9a8f9c53 Tree-SHA512: 68efacedbf72f67cf3dc0bb9927a698492cdc1b08df91ef6af863ad8828b78058a64e52d64d244a5b2966cb9e63797b2647d1bb222677bf83b26fca6e4b1dbf0
2022-04-28Merge bitcoin/bitcoin#18554: wallet: ensure wallet files are not reused ↵Andrew Chow
across chains 5f213213cb17429353ef7ec3e97b185af06d236f tests: add tests for cross-chain wallet use prevention (Seibart Nedor) 968765973b5bfde1ee4ad2fb5c19e24bce63ad0e wallet: ensure wallet files are not reused across chains (Seibart Nedor) Pull request description: This implements a proposal in #12805 and is a rebase of #14533. This seems to be a working approach, but I'm not sure why the `p2p_segwit.py` functional test needed a change, so I'll look into it more. ACKs for top commit: achow101: ACK 5f213213cb17429353ef7ec3e97b185af06d236f dongcarl: Code Review ACK 5f213213cb17429353ef7ec3e97b185af06d236f [deleted]: tACK https://github.com/bitcoin/bitcoin/pull/18554/commits/5f213213cb17429353ef7ec3e97b185af06d236f Tree-SHA512: 2c934300f113e772fc31c16ef5588526300bbc36e4dcef7d77bd0760c5c8f0ec77f766b1bed5503eb0157fa26dc900ed54d2ad1b41863c1f736ce5c1f3b67bec
2022-04-28Merge bitcoin/bitcoin#24984: wallet: ignore chainStateFlushed notifications ↵Andrew Chow
while attaching chain 2052e3aa9aa666bdc86dac370f1dd8fb978d3497 wallet: ignore chainStateFlushed notifications while attaching chain (Martin Zumsande) Pull request description: Fixes #24487 When a rescan is performed during `CWallet::AttachChain()` (e.g. when loading an old wallet) but this is interrupted by a shutdown signal, the wallet will currently stop the rescan, receive a `chainStateFlushed` signal, set the saved best block to the tip and shut down. At next startup, the rescan is not continued or repeated because of this. But some blocks have never been scanned by the wallet, which could lead to an incorrect balance. Fix this by ignoring `chainStateFlushed` notifications until the chain is attached. Since `CWallet::chainStateFlushed` is being manually called by `AttachChain()` anyway after finishing with the rescan, it is not a problem if intermediate notifications are ignored. Manual rescans started / aborted by the `rescanblockchain` / `abortrescan` RPCs are not affected by this. I didn't choose alternative ways of fixing this issue that would delay the validationinterface registration or change anything else about the handling of `blockConnected` signals for the reasons mentioned in [this existing comment](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2937-L2944). ACKs for top commit: achow101: ACK 2052e3aa9aa666bdc86dac370f1dd8fb978d3497 ryanofsky: Code review ACK 2052e3aa9aa666bdc86dac370f1dd8fb978d3497. This is a straightforward fix for the bug described in #24487 where a wallet could skip scanning blocks if is shut down in the middle of a sync and a chainStateFlushed notification was received during the sync. It would be nice to write a test for this but probably would be tricky to write. w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/24984/commits/2052e3aa9aa666bdc86dac370f1dd8fb978d3497 Tree-SHA512: a6186173d72b26bd4adbf2315e11af365004a723ea5565a0f7b868584dc47c321a6572eafaeb2420bd21eed1c7ad92b47e6218c5eb72313a3c6bee58364e2247
2022-04-28rpc: use GetBlockTime() for getblockchaininfo#timeJon Atack
2022-04-28blockstorage, refactor: pass GetFirstStoredBlock() start_block by referenceJon Atack
instead of by pointer, so as to not accept a nullptr.
2022-04-28blockstorage, refactor: make GetFirstStoredBlock() a member of BlockManagerJon Atack
instead of a global
2022-04-28test, bench: make prevector and checkqueue swap member functions noexceptJon Atack
Reason: A swap must not fail; when a class has a swap member function, it should be declared noexcept. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c84-a-swap-function-must-not-fail
2022-04-28Merge bitcoin/bitcoin#24956: Call CHECK_NONFATAL only once where neededMacroFake
fab34d392ca415c27605040dc0fc738016c9a0ca Call CHECK_NONFATAL only once where needed (MarcoFalke) Pull request description: Now that `CHECK_NONFATAL` is the identity function starting with commit b1c5991eebb916755be188f355ad36fe01a3f529, it can be called less often in places where it was called more than once on the same value. ACKs for top commit: jonatack: Review ACK fab34d392ca415c27605040dc0fc738016c9a0ca Tree-SHA512: ae221d7ee81f8d0be7ab21ce54d5d209e691df8a5c7f4a6f6db282453391904f87f533a2b7f85d6259827de8b85dacd9e0d9dbeecc4245a338247e0893ff3459
2022-04-28validation: make CScriptCheck and prevector swap member functions noexceptJon Atack
Reason: A swap must not fail; when a class has a swap member function, it should be declared noexcept. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c84-a-swap-function-must-not-fail
2022-04-28Merge bitcoin/bitcoin#24958: build: Fix macOS Apple M1 build with miniupnpc ↵laanwj
and libnatpmp. Again :) 165903406ecac363fc5101c2aa142aff75f76e8f build: Fix `AC_CHECK_HEADERS` and `AC_CHECK_LIB` for `libnatpmp` package (Hennadii Stepanov) 65cddf604c4b3c70e205b4cd32612a1ab8af9856 build: Fix `AC_CHECK_HEADERS` and `AC_CHECK_LIB` for `miniupnpc` package (Hennadii Stepanov) bbbcb9663853f3c034d1bdb621f366df6107970c build, refactor: Fix indentation (Hennadii Stepanov) Pull request description: Apparently, bitcoin/bitcoin#24391 broke the [ability](https://github.com/bitcoin/bitcoin/pull/22397) of the `configure` script to pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple M1. This PR fixes it. ACKs for top commit: promag: Tested ACK 165903406ecac363fc5101c2aa142aff75f76e8f jarolrod: tACK 165903406ecac363fc5101c2aa142aff75f76e8f Tree-SHA512: 93988f59f425890d60582b93d4ac5b2ad03011a5c6dbb44678a3ca591da7518c1c741bc1045b2c763bbe887947f32293b38d55fd7a96f09d2092ad34baa1db21
2022-04-28Merge bitcoin/bitcoin#24937: test: Remove previous release check in ↵laanwj
feature_taproot.py fafd67479aa770dac227622c893a9998fa78e99c test: Remove previous release check (MarcoFalke) Pull request description: Now that the commit (7c08d81e119570792648fe95bbacddbb1d5f9ae2) which changes taproot to be enforced for all blocks is sufficiently buried by other commits, and thus less likely to be reverted, it seems a good time to remove no longer needed test code. The `feature_taproot` functional test is cleaned up to no longer run against a previous release. Since previous releases are static and impossible to change, it is sufficient to run the test once against the release. Now that this is done, the check can be removed without decreasing test coverage. ACKs for top commit: laanwj: Concept and code review ACK fafd67479aa770dac227622c893a9998fa78e99c vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/24937/commits/fafd67479aa770dac227622c893a9998fa78e99c Tree-SHA512: fcb1a93f3bf9deb5f5c7327a7cd23be10ba09c9f4cbfa73ee2764a93c6ce7d6fa98ca32f2cf4023c20ab624aee601beec949fd02a57a3a658fdbd4be1a9ff338
2022-04-28Merge bitcoin/bitcoin#24322: [kernel 1/n] Introduce initial `libbitcoinkernel`fanquake
035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b build: Remove LIBTOOL_APP_LDFLAGS for bitcoin-chainstate (Cory Fields) 3f0595095dd6d230dc661641227937e3ab4ca8d3 docs: Add libbitcoinkernel_la_SOURCES explanation (Carl Dong) 94ad45deb257a95b4e98aa85da0371fb072fcd4c ci: Build libbitcoinkernel (Carl Dong) 26b2e7ffb3471a4712e5b9e50e066e0e3218f0dd build: Extract the libbitcoinkernel library (Carl Dong) 1df44dd20ca9e6e55eb353824b27d11bd1878c59 b-cs: Define G_TRANSLATION_FUN in bitcoinkernel.cpp (Carl Dong) 83a0bb7cc9907dbe089409ed5a417277ed63ed95 build: Separate lib_LTLIBRARIES initialization (Carl Dong) c1e16cb31f4d8edde8fea310011189b8b272cb07 build: Create .la library for bitcoincrypto (Carl Dong) 8bdfe057c796dde1cd2e5a37a73e87a879e9fe56 build: Create .la library for leveldb (Carl Dong) 05d1525b6d4412f68ff4c5460cd1daa6fb49969b build: Create .la library for crc32c (Carl Dong) 64caf944797bc35c3044fe5675389656f9511a41 build: Remove vestigial LIBLEVELDB_SSE42 (Carl Dong) 1392e8e2d8cfe4115f0a152aca16ffe3f0f4573a build: Don't add unrelated libs to LIBTEST_* (Carl Dong) Pull request description: Part of: #24303 This PR introduces a `libbitcoinkernel` static library linking in the minimal list of files necessary to use our consensus engine as-is. `bitcoin-chainstate` introduced in #24304 now will link against `libbitcoinkernel`. Most of the changes are related to the build system. Please read the commit messages for more details. ACKs for top commit: theuni: This may be my favorite PR ever. It's a privilege to ACK 035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b. Tree-SHA512: b755edc3471c7c1098847e9b16ab182a6abb7582563d9da516de376a770ac7543c6fdb24238ddd4d3d2d458f905a0c0614b8667aab182aa7e6b80c1cca7090bc
2022-04-28Merge bitcoin/bitcoin#24988: lint: Mention NONFATAL_UNREACHABLE in ↵fanquake
lint-assertions.py fa82a1ed833fd749849fa19267207b63e338d84d lint: Mention NONFATAL_UNREACHABLE in lint-assertions.py (MacroFake) Pull request description: Follow up to commit b1c5991eebb916755be188f355ad36fe01a3f529. Also remove empty newline added in that commit. ACKs for top commit: fanquake: ACK fa82a1ed833fd749849fa19267207b63e338d84d Tree-SHA512: cf398eceb135672137183bfa19ee57a82553a3dbcbce74db954c6fcd79f9606092cc0d8217610fe6cd67b7ef2d4f01d90329f0f568516d9b14aa2cd0f0715478
2022-04-28Merge bitcoin/bitcoin#24982: tests: Port `lint-all.sh` to `lint-all.py`laanwj
29f44fed36f45cb827c5e593ce52682a942bc296 Converting `lint-all.sh` to `lint-all.py`. (hiago) Pull request description: This PR is converting `test/lint/lint-all.sh` to `test/lint/lint-assertions.py`. It's an item of https://github.com/bitcoin/bitcoin/issues/24783. ACKs for top commit: laanwj: Tested ACK 29f44fed36f45cb827c5e593ce52682a942bc296 Tree-SHA512: 5427936aaa8e009613048448cbd79d9225675bdcadcf4fbb70fd091e0aab85e350ef1678da1b388f70d62ca16f40409409f90da3f6bbf057480522cd50fde811
2022-04-28Merge bitcoin/bitcoin#22564: refactor: Move mutable globals cleared in ↵MacroFake
`::UnloadBlockIndex` to `BlockManager` 7ab07e033237d6ea179a6a2c76575ed6bd01a670 validation: Prune UnloadBlockIndex and callees (Carl Dong) 7d99d725cdb5428ed25dc07c2d7fddf420da7786 validation: No mempool clearing in UnloadBlockIndex (Carl Dong) 572d8319272ae84a81d6bfd53dd9685585697f65 Clear {versionbits,warning}cache in ~Chainstatemanager (Carl Dong) eca4ca4d60599c9dbdd4e03a73beb33e9b44655a style-only: Use std::clamp for check_ratio, rename (Carl Dong) fe96a2e4bd87768df8001eb4117926a0977d876e style-only: Use for instead of when loading Chainstate (Carl Dong) 5921b863e39e5c3997895ffee1c87159e37a5d6f init: Reset mempool and chainman via reconstruction (Carl Dong) 6e747e80e7094df0b5bee1eed57e57e82015d0ee validation: default initialize and guard chainman members (Anthony Towns) 98f4bdae81804de17f125bd7c2cd8a48e850a6d2 refactor: Convert warningcache to std::array (Carl Dong) Pull request description: Fixes #22964 ----- This is a small part of the work to accomplish what I described in 972c5166ee685447a6d4bf5e501b07a0871fba85: ``` Over time, we should probably move these mutable global state variables into ChainstateManager or CChainState so it's easier to reason about their lifecycles. ``` `::UnloadBlockIndex` manually resets a subset of our mutable globals in addition to unloading the `ChainstateManager` and clearing the mempool. The need for this manual reset (AFAICT) arises out of the fact that many of these globals are closely related to the block index (hence `::UnloadBlockIndex`), and need to be reset with it. I've shot this "manual reset" gun at my foot several times while doing the de-globalize chainman work. Thankfully, now that we have a `BlockManager` class that owns the block index, these globals should be moved under that class so that they can live and die with the block index. These moves, along with making the block index non-heap-based, eliminates: 1. https://github.com/bitcoin/bitcoin/commit/3585b521392c5b2c855c3ba6dc9b7d2a171b3710 The need to reason about when we need to manually call `::UnloadBlockIndex` (this decision can at times seem almost arbitrary) 2. https://github.com/bitcoin/bitcoin/commit/f741623c25455c20bff9eb1ddd10a4ac84dc5655 The need to have an `::UnloadBlockIndex` or explicit `~ChainstateManager` at all ACKs for top commit: MarcoFalke: ACK 7ab07e033237d6ea179a6a2c76575ed6bd01a670 👘 ajtowns: ACK 7ab07e033237d6ea179a6a2c76575ed6bd01a670 ryanofsky: Code review ACK 7ab07e033237d6ea179a6a2c76575ed6bd01a670. This all looks good and simplifies things nicely. I left some minor suggestions below but feel free to ignore. Tree-SHA512: a36ee3fc122ce0b4e8d1c432662d7009df06264b724b793252978a1e409dde7a7ef1f78b9ade3f8bfb5388213f10ae2d058d57a7a46ae563e9034d7d33a52b69
2022-04-28rpc: Move signmessage RPC util to new fileMacroFake
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-04-28Remove cs_main from verifymessageMacroFake
2022-04-28Merge bitcoin/bitcoin#24831: tidy: add include-what-you-useMacroFake
9b0a13a2891641a3d12e525cee8ddddb1aa1bc73 tidy: Add include-what-you-use (fanquake) 74cd038e300bfbe2473295fc3b0c3a4f3e853a07 refactor: fix includes in src/init (fanquake) c79ad935f0412bac3e19a6b925efdb390eb00bd9 refactor: fix includes in src/compat (fanquake) Pull request description: We recently added a [`clang-tidy` job](https://github.com/bitcoin/bitcoin/blob/master/ci/test/00_setup_env_native_tidy.sh) to the CI, which generates a compilation database. We can leverage that now existing database to begin running [include-what-you-use](https://include-what-you-use.org/) over the codebase. This PR demonstrates using a mapping_file to indicate fixups / includes that may differ from IWYU suggestions. In this case, I've added some fixups for glibc includes that I've [upstreamed changes for](https://github.com/include-what-you-use/include-what-you-use/pull/1026): ```bash # Fixups / upstreamed changes [ { include: [ "<bits/termios-c_lflag.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] }, ] ``` The include "fixing" commits of this PR: * Adds missing includes. * Swaps C headers for their C++ counterparts. * Removes the pointless / unmaintainable `//for abc, xyz` comments. When using IWYU, if anyone wants to see / generate those comments, to see why something is included, it is trivial to do so (IWYU outputs them by default). i.e: ```cpp // The full include-list for compat/stdin.cpp: #include <compat/stdin.h> #include <poll.h> // for poll, pollfd, POLLIN #include <termios.h> // for tcgetattr, tcsetattr #include <unistd.h> // for isatty, STDIN_FILENO ``` TODO: - [ ] Qt mapping_file. There is one in the IWYU repo, but it's for Qt 5.11. Needs testing. - [ ] Boost mapping_file. There is one in the IWYU repo, but it's for Boost 1.75. Needs testing. I'm not suggesting we turn this on the for entire codebase, or immediately go-nuts refactoring all includes. However I think our dependency includes are now slim enough, and our CI infrastructure in place such that we can start doing this in some capacity, and just automate away include fixups / refactorings etc. ACKs for top commit: MarcoFalke: review ACK 9b0a13a2891641a3d12e525cee8ddddb1aa1bc73 jonatack: ACK 9b0a13a2891641a3d12e525cee8ddddb1aa1bc73 reviewed changes and run CI output in https://cirrus-ci.com/task/4750910332076032 Tree-SHA512: 00beab5a5f2a6fc179abf08321a15391ecccaa91ab56f3c50c511e7b29a0d7c95d8bb43eac2c31489711086f6f77319d43d803cf8ea458e7cd234a780d9ae69e
2022-04-28Merge bitcoin/bitcoin#25011: tests: Do not always create a descriptor wallet ↵MacroFake
in wallet_createwallet 786b3a7c44437aedb46a7e21ee54a18ba2802d9b tests: Do not always create a descriptor wallet in wallet_createwallet (Andrew Chow) Pull request description: The createwallet test for some invalid parameters incorrectly always creates a descriptor wallet. This is unnecessary and also breaks the test when bdb is not compiled in. Fixes #25007 ACKs for top commit: jacobpfickes: ACK 786b3a7c44437aedb46a7e21ee54a18ba2802d9b Tree-SHA512: 97b0953a08adf83d5ea84cac2651253d790b43d606a2f746dd45d3ccd1fb576bab63e3835e3de592715ef8a5cb133e6f19a3ab810fedf4684072143c3cb578d4
2022-04-27build: Remove LIBTOOL_APP_LDFLAGS for bitcoin-chainstateCory Fields
See added comment. Note that this won't actually have any effect until we add the mingw-w64 DLL fix since LIBTOOL_APP_LDFLAGS is undefined for other platforms.
2022-04-27docs: Add libbitcoinkernel_la_SOURCES explanationCarl Dong
2022-04-27ci: Build libbitcoinkernelCarl Dong
2022-04-27build: Extract the libbitcoinkernel libraryCarl Dong
I strongly recommend reviewing with the following git-diff flags: --patience --color-moved=dimmed-zebra Extract out a libbitcoinkernel library linking in all files necessary for using our consensus engine as-is. Link bitcoin-chainstate against it. See previous commit "build: Add example bitcoin-chainstate executable" for more context. We explicitly specify -fvisibility=default, which effectively overrides the effects of --enable-reduced-exports since libbitcoinkernel requires default symbol visibility When compiling for mingw-w64, specify -static in both: - ..._la_CXXFLAGS so that libtool will avoid building two versions of each object (one PIC, one non-PIC). We just need the one that is suitable for static linking. - ..._la_LDFLAGS so that libtool will create a static library. If we don't specify this, then libtool will prefer the non-static PIC version of the object, which is built with -DDLL_EXPORT -DPIC for mingw-w64 targets. This can cause symbol resolution problems when we link this library against an executable that does specify -all-static, since that will be built without the -DDLL_EXPORT flag. Unfortunately, this means that for mingw-w64 we can only build a static version of the library for now. This will be fixed. However, on other targets, the shared library creation works fine. ----- Note to users: You need to either specify: --enable-experimental-util-chainstate or, --with-experimental-kernel-lib To build the libbitcionkernel library. See the configure help for more details. build shared libbitcoinkernel where we can
2022-04-27tests: Do not always create a descriptor wallet in wallet_createwalletAndrew Chow
The createwallet teswt for some invalid parameters incorrectly always creates a descriptor wallet. This is unnecessary and also breaks the test when bdb is not compiled in.
2022-04-27Merge bitcoin/bitcoin#18642: Use std::chrono for the time to rotate ↵MacroFake
destination of addr messages + tests 2ff8f4dd81dc484fe38ddd9db63cc8fd30192245 Add tests for addr destination rotation (Gleb Naumenko) 77ccb7fce15e340080f14c7626cf3dc63fcdee88 Use std::chrono for salting when randomizing ADDR destination (Gleb Naumenko) Pull request description: We currently assign a destination peer for relaying particular addresses of nodes every 24 hours, and then rotate. This is done for rate-limiting (ultimately for privacy leak reduction I think?). Before this change, 24 hours was defined as uint. I replaced it with std::chrono, which is mockable and type-safe. Also added couple tests for this behavior. ACKs for top commit: jonatack: ACK 2ff8f4dd81dc484fe38ddd9db63cc8fd30192245 Tree-SHA512: 16f703ef3ffee13ce3afa82ca7b4baa27308af18cd2eececdce5565badfb68656a2ad9c4594b73772e4bfa99b3fb15f8e4089c1cb4be98c0bae6730a9d2f8a25
2022-04-27Crash debug builds on PCKG_MEMPOOL_ERRORMacroFake
2022-04-27Merge bitcoin/bitcoin#25001: Modernize util/strencodings and util/string: ↵laanwj
`string_view` and `optional` fa7078d84fc2858a466bc1a85404f821df682538 scripted-diff: Rename ValidAsCString to ContainsNoNUL (MacroFake) e7d2fbda63c346ae88767c3f8d4db3edeae2dc0b Use std::string_view throughout util strencodings/string (Pieter Wuille) 8ffbd1412d887535ce5eb613884858c319bd12be Make DecodeBase{32,64} take string_view arguments (Pieter Wuille) 1a72d62152bfdd7c5c2b2704b679f894e7d35e37 Generalize ConvertBits to permit transforming the input (Pieter Wuille) 78f3ac51b7d073d12da6a3b9b7d80d91e04ce3a7 Make DecodeBase{32,64} return optional instead of taking bool* (Pieter Wuille) a65931e3ce66d87b8f83d67ecdbb46f137e6a670 Make DecodeBase{32,64} always return vector, not string (Pieter Wuille) a4377a0843636eae0aaf698510fc6518582545db Reject incorrect base64 in HTTP auth (Pieter Wuille) d648b5120b2fefa9e599898bd26f05ecf4428fac Make SanitizeString use string_view (Pieter Wuille) 963bc9b576f0a62caffede2ce32830aef3473995 Make IsHexNumber use string_view (Pieter Wuille) 40062997f223d88d4f92aaae4622a31476686163 Make IsHex use string_view (Pieter Wuille) c1d165a8c2678c31aced5e1d46231d9996b0774a Make ParseHex use string_view (Pieter Wuille) Pull request description: Make use of `std::string_view` and `std::optional` in the util/{strencodings, string} files. This avoids many temporary string/vector objects being created, while making the interface easier to read. Changes include: * Make all input arguments in functions in util/strencodings and util/string take `std::string_view` instead of `std::string`. * Add `RemovePrefixView` and `TrimStringView` which also *return* `std::string_view` objects (the corresponding `RemovePrefix` and `TrimString` keep returning an `std::string`, as that's needed in many call sites still). * Stop returning `std::string` from `DecodeBase32` and `DecodeBase64`, but return vectors. Base32/64 are fundamentally algorithms for encoding bytes as strings; returning `std::string` from those (especially doing it conditionally based on the input arguments/types) is just bizarre. * Stop taking a `bool* pf_invalid` output argument pointer in `DecodeBase32` and `DecodeBase64`; return an `std::optional` instead. * Make `DecodeBase32` and `DecodeBase64` more efficient by doing the conversion from characters to integer symbols on-the-fly rather than through a temporary vector. ACKs for top commit: MarcoFalke: re-ACK fa7078d84fc2858a466bc1a85404f821df682538 only change is rebase and adding a scripted-diff 🍲 martinus: Code review ACK fa7078d84fc2858a466bc1a85404f821df682538, found no issue laanwj: Code review ACK fa7078d84fc2858a466bc1a85404f821df682538 sipa: utACK fa7078d84fc2858a466bc1a85404f821df682538 (as far as the commit that isn't mine goes) Tree-SHA512: 5cf02e541caef0bcd100466747664bdb828a68a05dae568cbcd0632a53dd3a4c4e85cd8c48ebbd168d4247d5c9666689c16005f1c8ad75b0f057d8683931f664
2022-04-27validation: Prune UnloadBlockIndex and calleesCarl Dong
In previous commits in this patchset, we've made sure that every Unload/UnloadBlockIndex member function resets its own members, and does not reach out to globals. This means that their corresponding classes' default destructors can now replace them, and do an even more thorough job without the need to be updated for every new member variable. Therefore, we can remove them, and also remove UnloadBlockIndex since that's not used anymore. Unfortunately, chainstatemanager_loadblockindex relies on CChainState::UnloadBlockIndex, so that needs to stay for now.
2022-04-27validation: No mempool clearing in UnloadBlockIndexCarl Dong
The only caller that uses this is ~ChainTestingSetup() where we immediately destroy the mempool afterwards.