aboutsummaryrefslogtreecommitdiff
path: root/src/init.cpp
AgeCommit message (Collapse)Author
2022-06-15Merge bitcoin/bitcoin#25338: scripted-diff: Avoid incompatibility with CMake ↡MacroFake
AUTOUIC feature 018d70b58726b361b6951e0e6de04f13eb97a89d scripted-diff: Avoid incompatibility with CMake AUTOUIC feature (Hennadii Stepanov) Pull request description: Working on [migration](https://github.com/hebasto/bitcoin/pull/3) from Autotools to CMake build system, I found that our current code base needs to be adjusted. CMake [allows](https://cmake.org/cmake/help/latest/prop_tgt/AUTOUIC.html) to > handle the Qt `uic` code generator automatically When using this feature, statements like `#include "ui_<ui_base>.h"` are processed in a special way. The `node/ui_interface.h` unintentionally breaks this feature. Of course, it is possible to provide a list of source files to be excluded from `AUTOUIC`. But, unfortunately, this approach does not work for the `qt/sendcoinsdialog.cpp` source file, where there are both https://github.com/bitcoin/bitcoin/blob/b71d37da2c8c8d2a9cef020731767a6929db54b4/src/qt/sendcoinsdialog.cpp#L10 and https://github.com/bitcoin/bitcoin/blob/b71d37da2c8c8d2a9cef020731767a6929db54b4/src/qt/sendcoinsdialog.cpp#L24 ACKs for top commit: MarcoFalke: cr ACK 018d70b58726b361b6951e0e6de04f13eb97a89d ryanofsky: Code review ACK 018d70b58726b361b6951e0e6de04f13eb97a89d furszy: Code review ACK 018d70b5 Tree-SHA512: 4fc83f2e5a82c8ab15c3c3d68f48b9863c47b96c0a66b6276b9b4dfc6063abffd73a16382acfe116553487b3ac697dbde2d9ada1b92010c5d8f8c6aa06f56428
2022-06-14Merge bitcoin/bitcoin#25306: logging: add LogPrintfCategory to log ↡laanwj
unconditionally with category ecff20db286e2f5d3afe32cfaae72de69d34d23c logging: use LogPrintfCategory rather than a manual category (Jon Atack) eb8aab759fb15824a5dd3004e689d0eb5b884a32 logging: add LogPrintfCategory to log unconditionally with category (Jon Atack) Pull request description: These are the next two commits from #25203. - Add `LogPrintfCategory` to log unconditionally while prefixing the output with the passed category name. Add documentation and a unit test, and update the `lint-logs.py` and `lint-format-strings.py` scripts. - Replace the log messages that manually print a category, with `LogPrintfCategory`. In upcoming commits, it will likely be used in many other cases, such as to replace `LogPrintf` where it makes sense. ACKs for top commit: klementtan: Code Review ACK ecff20db286e2f5d3afe32cfaae72de69d34d23c laanwj: Code review ACK ecff20db286e2f5d3afe32cfaae72de69d34d23c brunoerg: ACK ecff20db286e2f5d3afe32cfaae72de69d34d23c Tree-SHA512: ad3a82835254f7606efcd14b88f3d9072f1eb9b25db1321ed38ef6a4ec60efd555d78f5e19d93736f2f8500251d06f8beee9d694a153f24bf5cce3590a2a45a5
2022-06-14scripted-diff: Avoid incompatibility with CMake AUTOUIC featureHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i "s|node/ui_interface|node/interface_ui|g" $(git grep -l "node/ui_interface" ./src) git mv src/node/ui_interface.cpp src/node/interface_ui.cpp git mv src/node/ui_interface.h src/node/interface_ui.h sed -i "s|BITCOIN_NODE_UI_INTERFACE_H|BITCOIN_NODE_INTERFACE_UI_H|g" src/node/interface_ui.h -END VERIFY SCRIPT-
2022-06-10Merge bitcoin/bitcoin#24931: Strengthen thread safety assertionsMacroFake
ce893c0497fc9b8ab9752153dfcc77c9f427545e doc: Update developer notes (Anthony Towns) d2852917eecad6ab422a7b2c9892d351a7f0cc96 sync.h: Imply negative assertions when calling LOCK (Anthony Towns) bba87c0553780eacf0317fbfec7330ea27aa02f8 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns) a559509a0b8cade27199740212d7b589f71a0e3b sync.h: Add GlobalMutex type (Anthony Towns) be6aa72f9f8d50b6b5b19b319a74abe7ab4099ff qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns) f24bd45b37e1b2d19e5a053dbfefa30306c1d41a net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns) Pull request description: This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions. This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex` to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`). ACKs for top commit: MarcoFalke: review ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e 🐦 hebasto: ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
2022-06-08logging: use LogPrintfCategory rather than a manual categoryJon Atack
Here we update only the log messages that manually print a category. In upcoming commits, LogPrintCategory will likely be used in many other cases, such as to replace `LogPrintf` where it makes sense.
2022-06-02kernel: SanityChecks: Return an error structCarl Dong
This reduces libbitcoinkernel's coupling with ui_interface and translation.
2022-06-02Move init::SanityCheck to kernel::SanityCheckCarl Dong
2022-06-02init: Initialize globals with kernel::Context's lifeCarl Dong
...instead of explicitly calling init::{Set,Unset}Globals. Cool thing about this is that in both the testing and bitcoin-chainstate codepaths, we no longer need to explicitly unset globals. The kernel::Context goes out of scope and the globals are unset "automatically". Also construct kernel::Context outside of AppInitSanityChecks()
2022-05-28compat: remove glibcxx sanity checksfanquake
These checks were added in #4339, (see also #4081), to test our back-compat stubs, however, those stubs no-longer exist (#22930), meaning that these checks are now just testing some specific standard library behaviour, without a particular rationale, or reason, compared to any other standard library functions we use. There has also been some discussion about the sanity checks in the context of the libbitcoinkernel refactoring, see https://github.com/bitcoin/bitcoin/pull/25065#discussion_r880668218. Removing the checks removes the need to worry about atleast the glibcxx checks. Also remove the list of check from the doc in init.h, because it is incomplete, and anyone who wants to know what checks are included can look at the function.
2022-05-26Merge bitcoin/bitcoin#15936: interfaces: Expose settings.json methods to GUIMacroFake
f9fdcec7e932843a91ddf7f377e00bd2a6efb82a settings: Add resetSettings() method (Ryan Ofsky) 77fabffef4ea840ee15c97061048fe8443d74658 init: Remove Shutdown() node.args reset (Ryan Ofsky) 0e55bc6e7fe439404dc56093a0949395dae51e6b settings: Add update/getPersistent/isIgnored methods (Ryan Ofsky) Pull request description: Add `interfaces::Node` `updateSetting`, `forceSetting`, `resetSettings`, `isSettingIgnored`, and `getPersistentSetting` methods so GUI is able to manipulate `settings.json` file and use and modify node settings. (Originally this PR also contained GUI changes to unify bitcoin-qt and bitcoind persistent settings and call these methods, but the GUI commits have been dropped from this PR and moved to bitcoin-core/gui/pull/602) ACKs for top commit: vasild: ACK f9fdcec7e932843a91ddf7f377e00bd2a6efb82a hebasto: re-ACK f9fdcec7e932843a91ddf7f377e00bd2a6efb82a, only a function renamed since my recent [review](https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-979324357). Tree-SHA512: 4cac853ee29be96d2ff38404165b9dfb7c622b2a9c99a15979596f3484ffde0da3d9c9c372677dff5119ca7cffa6383d81037fd9889a29cc9285882a8dc0c268
2022-05-21refactor: Remove defunct attributes.h includesBen Woosley
Since the removal of NODISCARD in 81d5af42f4dba5b68a597536cad7f61894dc22a3, the only attributes def is LIFETIMEBOUND, and it's included in many more places that it is used. This removes all includes which do not have an associated use of LIFETIMEBOUND, and adds it to the following files, due to their use of the same: * src/validationinterface.h * src/script/standard.h
2022-05-20Add ChainstateManager::m_adjusted_time_callbackCarl Dong
This decouples validation.cpp from netaddress.cpp (transitively, timedata.cpp, and asmap.cpp). This is important for libbitcoinkernel as: - There is no reason for the consensus engine to be coupled with netaddress, timedata, and asmap - Users of libbitcoinkernel can now easily supply their own std::function that provides the adjusted time. See the src/Makefile.am changes for some satisfying removals.
2022-05-20Add ChainstateManagerOpts, using as ::OptionsCarl Dong
[META] Although it seems like we don't need it for just one option, we're going to introduce another member to this struct *in the next commit*. In future patchsets for libbitcoinkernel decoupling it from ArgsManager, even more members will be added here.
2022-05-21scripted-diff: Convert global Mutexes to GlobalMutexesAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i -E -e '/^([a-z]+ )?Mutex [a-z]/ s/Mutex/GlobalMutex/' $(git grep -lE '^([a-z]+ )?Mutex [a-z]') -END VERIFY SCRIPT-
2022-05-20Merge bitcoin/bitcoin#25168: refactor: Avoid passing params where not neededMacroFake
fa1b76aeb064b315a3767a8f59836ca18aeb117e Do not call global Params() when chainman is in scope (MacroFake) fa30234be81b6f49ae8150478a9255daa1611083 Do not pass CChainParams& to PeerManager::make (MacroFake) fafe5c0ca2927642cbcec63ac73994737e1653d6 Do not pass CChainParams& to BlockAssembler constructor (MacroFake) faf012b438b451dced785e7f031e07c0c55665e1 Do not pass Consensus::Params& to Chainstate helpers (MacroFake) fa4ee53dca5ccf1b87f019f372ffc10528add943 Do not pass time getter to Chainstate helpers (MacroFake) Pull request description: It seems confusing to pass chain params, consensus params, or a time function around when it is not needed. Fix this by: * Inlining the passed time getter function. I don't see a use case why this should be mockable. * Using `chainman.GetConsensus()` or `chainman.GetParams()`, where possible. ACKs for top commit: promag: Code review ACK fa1b76aeb064b315a3767a8f59836ca18aeb117e. vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25168/commits/fa1b76aeb064b315a3767a8f59836ca18aeb117e Tree-SHA512: 1abff5cba4b4871d97f17dbcdf67bc9255ff21fa4150a79a74e39b28f0610eab3e7dee24d56872dd6e111f003b55e288958cdd467e6218368d896f191e4ec9cd
2022-05-20Merge bitcoin/bitcoin#24830: init: Allow -proxy="" setting valuesMacroFake
1d4122dfefcb0a33c3d5bf7bbe2c7cd7e09d3764 init: Allow -proxy="" setting values (Ryan Ofsky) Pull request description: This drops the `No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>` error when a empty `-proxy=` command line argument, `bitcoin.conf` value, or `settings.json` value is specified, and just makes bitcoin connect and listen normally in these cases. The error was originally added in https://github.com/bitcoin/bitcoin/pull/20003 to prevent a bare `-proxy` command line argument with no assignment from clearing proxy settings. But it was implemented in an overbroad way breaking empty `-proxy=` assignments as well. The motivation for this change is to prevent a GUI bug that happens with https://github.com/bitcoin/bitcoin/pull/15936, reported in https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-937685759 by vasild, that happens after a proxy setting is enabled and disabled in the GUI. But this change also makes sense on its own to remove a potentially confusing error message. ACKs for top commit: hebasto: re-ACK 1d4122dfefcb0a33c3d5bf7bbe2c7cd7e09d3764, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/24830#pullrequestreview-941255672). Tree-SHA512: 753adfce199ed078a6cd9e0ea78e76c0b14070f8fcfe2a4632cd0c6dfe6b4e135ddffbe11a97e5e30520ea9e5bda00bad1493cbaef74cf425aa8613249167f53
2022-05-19init: Remove Shutdown() node.args resetRyan Ofsky
This commit removes the `node.args = nullptr` assignment in the Shutdown() function. Clearing node.args there never made sense because it made the Shutdown() function not idempotent, making it fragile and causing issues like https://github.com/bitcoin/bitcoin/issues/23186. The assignment also causes segfaults in GUI unit tests when a new node().initParameterInteraction() call is added in OptionsModel to apply to Qt settings (happens because AppTests calls Shutdown() which sets node.args to null, and OptionTests runs after AppTests and then needs node.args not to be null.)
2022-05-18rpc: Put undocumented JSON failure mode behind a runtime flagSuhail Saqan
rpc: Put undocumented JSON failure mode behind a runtime flag
2022-05-18Do not call global Params() when chainman is in scopeMacroFake
2022-05-18Do not pass CChainParams& to PeerManager::makeMacroFake
2022-05-18Do not pass Consensus::Params& to Chainstate helpersMacroFake
2022-05-18Do not pass time getter to Chainstate helpersMacroFake
2022-05-13Merge bitcoin/bitcoin#24595: deploymentstatus: move g_versionbitscache ↡MacroFake
global to ChainstateManager bb5c24b120a3ac7df367a1c5d9b075ca564efb5f validation: move g_versionbitscache into ChainstateManager (Anthony Towns) eca22c726ac48b4216bb68cc0f0bbd655c43ac12 test/versionbits: make versionbitscache a parameter (Anthony Towns) d603f1d8a7cdc0a158ed80ade8a843b61b6ad08e deploymentstatus: make versionbitscache a parameter (Anthony Towns) 78adef17536edef833a0bfca06b61ce28120e486 refactor: use chainman instead of chainParams for DeploymentActive* (Anthony Towns) deffe0df6c36225bada18603b5a840139f030f2c deploymentstatus: allow chainman in place of consensusParams (Anthony Towns) eaa2e3f25cefbd1b9a1214102f88dbfa8109d244 validation: move UpdateUncommittedBlockStructures and GenerateCoinbaseCommitment into ChainstateManager (Anthony Towns) 5c67e84d37d452e9186a6357e5405fabeff241c7 validation: replace ::Params() calls with chainstate/chainman member (Anthony Towns) 38860f93b680f152fc6fc3d9ae574a4c0659e775 validation: remove redundant CChainParams params from ChainstateManager methods (Anthony Towns) 69675ea4e73dcf5e9dd0f94802bd3463e4262081 validation: add CChainParams to ChainstateManager (Anthony Towns) Pull request description: Gives `ChainstateManager` a reference to the `CChainParams` its working on, and simplifies some of the functions that would otherwise take that as a parameter. Removes the `g_versionbitscache` global by moving it into `ChainstateManager`. ACKs for top commit: dongcarl: reACK bb5c24b120a3ac7df367a1c5d9b075ca564efb5f MarcoFalke: review ACK bb5c24b120a3ac7df367a1c5d9b075ca564efb5f πŸ“™ Tree-SHA512: 3fa74905e5df561e3e74bb0b8fce6085c5311e6633e7d74c0fb0c82a907f5bbb1fd4ebc5d11d4f0b1c019bb51eabb9f6e4bcc4652a696d36a5878c807b85f121
2022-05-12Merge bitcoin/bitcoin#24925: refactor: make GetRand a template, remove ↡MacroFake
GetRandInt ab1ea29ba1b8379a21fabd3dc859552c470a6421 refactor: make GetRand a template, remove GetRandInt (pasta) Pull request description: makes GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided. This simplifies a lot of code from GetRand(std::numeric_limits<uint64_t>::max() -> GetRand<uint64_t>() ACKs for top commit: laanwj: Code review ACK ab1ea29ba1b8379a21fabd3dc859552c470a6421 Tree-SHA512: db5082a0e21783389f1be898ae73e097b31ab48cab1a2c0e29348a4adeb545d4098193aa72a547c6baa6e8205699aafec38d6a27b3d65522fb3246f91b4daae9
2022-05-10validation: add CChainParams to ChainstateManagerAnthony Towns
2022-05-05Wrap boost::replace_allMacroFake
2022-05-04Merge bitcoin/bitcoin#22235: script: add script to generate example bitcoin.conflaanwj
b42643c2537ffbe99d6d94fb1fb3b7f9d5234f93 doc: update init.cpp -conf help text (josibake) 970b9987ad5bcb72e581c40a7cdd408d94a48c81 doc: update devtools, release-process readmes (josibake) 50635d27b45d125b6264ac2abfbd6a1129c7228f build: include bitcoin.conf in build outputs (josibake) 6aac946f49aea243de1dc50631bb72f0186bbf58 doc: update bitcoin-conf.md (Josiah Baker) 1c7e820ded0846ef6ab4be9616b0de452336ef64 script: add script to generate example bitcoin.conf (josibake) b483084d866c16d97a34251ae652bac94f85f61d doc: replace bitcoin.conf with placeholder file (josibake) Pull request description: create a script for parsing the output from `bitcoind --help` to create an example conf file for new users ## problem per #10746 , `bitcoin.conf` not being put into the data directory during installation causes some confusion for users when running bitcoin. in the discussion on the issue, one proposed solution was to have an example config file and instruct users to `cp` it into their data directory after startup. in addition to #10746 , there have been other requests for a "skeleton config file" (https://github.com/bitcoin/bitcoin/issues/19641) to help users get started with configuring bitcoind. the main issue with an example config file is that it creates a second source of truth regarding what options are available for configuring bitcoind. this means any changes to the options (including the addition or removal of options) would have to be updated for the command line and also updated in the example file. this PR addresses this issue by providing a script to generate an example file directly from the `bitcoind --help` on-demand by running `contrib/devtools/gen-bitcoin-conf.sh`. this solution was originally proposed on #10746 and would also solve #19641 . this guarantees any changes made to the command-line options or the command-line options help would also be reflected in the example file after compiling and running the script. the main purpose of this script is to generate a config file to be included with releases, same as `gen-manpages.sh`. this ensures every release also includes an up-to-date, full example config file for users to edit. the script is also available for users who compile from source for generating an example config for their compiled binary. ## special considerations this removes the `bitcoin.conf` example file from the repo as it is now generated by this script. the original example file did contain extra text related to how to use certain options but going forward all option help docs should be moved into `init.cpp` this also edits `init.cpp` to have the option help indicate that `-conf` is not usable from the config file. this is similar to how `-includeconf` 's help indicates it cannot be used from the command line ACKs for top commit: laanwj: Tested and code review ACK b42643c2537ffbe99d6d94fb1fb3b7f9d5234f93 Tree-SHA512: 4546e0cef92aa1398da553294ce4712d02e616dd72dcbe0b921af474e54f24750464ec813661f1283802472d1e8774e634dd1cc26fbf1f13286d3e0406c02c09
2022-05-04Merge bitcoin/bitcoin#24933: util: Replace non-threadsafe strerrorlaanwj
e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c test: Add `strerror` to locale-dependence linter (laanwj) f00fb1265a8bc26e1612c771173325dbe49b3612 util: Increase buffer size to 1024 in SysErrorString (laanwj) 718da302c7b11b375042c3000d421fd93348c199 util: Refactor SysErrorString logic (laanwj) e7f2f77756d33c6be9c8998a575b263ff2d39270 util: Use strerror_s for SysErrorString on Windows (laanwj) 46971c6dbfbc39ebbc74ab1ed8c00edc12859373 util: Replace non-threadsafe strerror (laanwj) Pull request description: Some uses of non-threadsafe `strerror` have snuck into the code since they were removed in #4152. Add a wrapper `SysErrorString` for thread-safe strerror alternatives (with code from `NetworkErrorString`) and replace all uses of `strerror` with this. Edit: I've also added a commit that refactors the code so that buf[] is never read at all if the function fails, making some fragile-looking code unnecessary. Edit2: from the linux manpage: ``` ATTRIBUTES For an explanation of the terms used in this section, see attributes(7). β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚Interface β”‚ Attribute β”‚ Value β”‚ β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ β”‚strerror() β”‚ Thread safety β”‚ MT-Unsafe race:strerror β”‚ β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ … β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ β”‚strerror_r(), β”‚ Thread safety β”‚ MT-Safe β”‚ β”‚strerror_l() β”‚ β”‚ β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ ``` As the function can be called from any thread at any time, using a non-thread-safe function is unacceptable. ACKs for top commit: jonatack: ACK e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c Tree-SHA512: 20e71ebb9e979d4e1d8cafbb2e32e20c2a63f09115fe72cdde67c8f80ae98c531d286f935fd8a6e92a18b72607d7bd3e846b2d871d9691a6036b0676de8aaf25
2022-05-04doc: update init.cpp -conf help textjosibake
update help to reflect this option cannot be used from the config file
2022-04-28util: Replace non-threadsafe strerrorlaanwj
Some uses of non-threadsafe `strerror` have snuck into the code since they were removed in #4152. Add a wrapper `SysErrorString` for thread-safe strerror alternatives and replace all uses of `strerror` with this.
2022-04-27style-only: Use std::clamp for check_ratio, renameCarl Dong
2022-04-27style-only: Use for instead of when loading ChainstateCarl Dong
It's a bit clearer and restricts the scope of fLoaded
2022-04-27init: Reset mempool and chainman via reconstructionCarl Dong
Fixes https://github.com/bitcoin/bitcoin/issues/22964 Previously, we used UnloadBlockIndex() in order to reset node.mempool and node.chainman. However, that has proven to be fragile (see https://github.com/bitcoin/bitcoin/issues/22964), and requires UnloadBlockIndex and its callees to be updated manually for each member that's introduced to the mempool and chainman classes. In this commit, we stop using the UnloadBlockIndex function and we simply reconstruct node.mempool and node.chainman. Since PeerManager needs a valid reference to both node.mempool and node.chainman, we also move PeerManager's construction via `::make` to after the chainstate activation sequence is complete. There are no more callers to UnloadBlockIndex after this commit, so it and its sole callees can be pruned.
2022-04-26Merge bitcoin/bitcoin#21726: Improve Indices on pruned nodes via prune blockersfanquake
71c3f0356c01521a95c64fba1e7375aea6286bb0 move-only: Rename index + pruning functional test (Fabian Jahr) de08932efa953e9a237cbf879460488ad8947411 test: Update test for indices on pruned nodes (Fabian Jahr) 825d19839bf71245306d4c8edde040e5941caa46 Index: Allow coinstatsindex with pruning enabled (Fabian Jahr) f08c9fb0c6a799e3cb75ca5f763a746471625beb Index: Use prune locks for blockfilterindex (Fabian Jahr) 2561823531c25e1510c107eb41de944b00444ce0 blockstorage: Add prune locks to BlockManager (Fabian Jahr) 231fc7b035481f748159968353c1cab81354e843 refactor: Introduce GetFirstStoredBlock helper function (Fabian Jahr) Pull request description: # Motivation The main motivation of this change and only behavior change noticeable by user is to allow running `coinstatsindex` on pruned nodes as has been requested [here for example](https://twitter.com/benthecarman/status/1388170854140452870?s=20). # Background `coinstatsindex` on pruned nodes can be enabled in a much simpler than it is done here but it comes with downside. The ability to run `blockfilterindex`on pruned nodes was added in #15946 but it also added the `blockfilterindex` as a dependency to `validation` and it introduced two new circular dependencies. Enabling `coinstatsindex` on pruned nodes in a similar way would add it as a dependency as well and introduce another circular dependency. Instead, this PR introduces a `m_prune_blockers` map to `BlockManager` as a flexible approach to block pruning. Entities like `blockfilterindex`, for example, can add a key and a height to block pruning over that height. These entities need to update that value to allow more pruning when they are ready. # Alternative approach Upon completing the first draft of this PR I found #19463 as an alternative that follows the same but follows a very different approach. I am listing the main differences here as I see them: - Usage of globals - Blocks pruning with a start and a stop height - Can persist blockers across restarts - Blockers can be set/unset via RPCs Personally, I don't think any of these are necessary to be added here but if the general approach or specific features are more appealing to reviewers I am happy to change to a solution based on that PR or port over specific parts of it here. ACKs for top commit: mzumsande: Code review ACK 71c3f0356c01521a95c64fba1e7375aea6286bb0 ryanofsky: Code review ACK 71c3f0356c01521a95c64fba1e7375aea6286bb0. Changes since last review: just tweaking comments and asserts, and rebasing w0xlt: tACK https://github.com/bitcoin/bitcoin/pull/21726/commits/71c3f0356c01521a95c64fba1e7375aea6286bb0 on signet. Tree-SHA512: de7efda08b44aa31013fbebc47a02cd2de32db170b570f9643e1f013fee0e8e7ca3068952d1acc6e5e74a70910735c5f263437981ad73df841ad945b52d36b71
2022-04-26init: Allow -proxy="" setting valuesRyan Ofsky
This drops the `No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>` error when a empty `-proxy=` command line argument, `bitcoin.conf` value, or `settings.json` value is specified, and just makes bitcoin connect and listen normally in these cases. The error was originally added in https://github.com/bitcoin/bitcoin/pull/20003 to prevent a bare `-proxy` command line argument with no assignment from clearing proxy settings. But it was implemented in an overbroad way breaking empty `-proxy=` assignments as well. The motivation for this change is to prevent a GUI bug that happens with https://github.com/bitcoin/bitcoin/pull/15936, reported in https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-937685759 by vasild, that happens after a proxy setting is enabled and disabled in the GUI. But this change also makes sense on its own to remove a potentially confusing error message.
2022-04-26Merge bitcoin/bitcoin#24789: init, index: disallow indexes when running ↡fanquake
reindex-chainstate dac44fc06ff9938d070aa5221d106a7f31da9d81 init: disallow reindex-chainstate with optional indexes (Martin Zumsande) 62e14285f95d4ecb9528813acca975640dd7c598 doc: Add note that -reindex will rebuild optional indexes (Martin Zumsande) Pull request description: When started together with `-reindex-chainstate`, currently coinstatsindex gets corrupted and the blockfilterindex flatfiles duplicated. See the OP of #24630 for more a more detailed explanation on why this happens. This is an alternative to #24630 which does not wipe and rebuild the indexes but returns an `InitError` when they are activated, thus requiring the user to deactivate them temporarily until the `-reindex-chainstate` run is finished. This also disallows `-reindex-chainstate` in combination with `-txindex`, which is not leading to corruption, but currently still rebuilds the index unnecessarily and unexpectedly. As a long-term goal, it would be desirable to have the indexes tolerate `reindex-chainstate` by ignoring their `BlockConnected` notifications (there is discussion in #24630 about this) or possibly move `reindex-chainstate` option into a `bitcoin-chainstate` executable, which could also solve the problem. But these would be larger projects - until then, it might be better to disallow the interaction than having corrupted indexes. The first commit adjusts the `-reindex` doc to mention that this option does rebuild all active indexes. ACKs for top commit: ryanofsky: Code review ACK dac44fc06ff9938d070aa5221d106a7f31da9d81. Just fixed IsArgSet call and edited error messages since last review Tree-SHA512: c1abf7d350648ae227c3fd6c95d9a54c3bac9de70915275dea1c87cca6d9a76a056c0e306d95ef8cfe4df1f8525b418e0e7a4f52ded3be464041c0dc297f8930
2022-04-25Index: Allow coinstatsindex with pruning enabledFabian Jahr
2022-04-24init: disallow reindex-chainstate with optional indexesMartin Zumsande
It currently leads to corruption (coinstatsindex) or data duplication (blockfilterindex), so disable it.
2022-04-22refactor: make GetRand a template, remove GetRandIntpasta
2022-04-22Merge bitcoin/bitcoin#22910: net: Encapsulate asmap in NetGroupManagerfanquake
36f814c0e84d009c0e0aa26981a20ac4cf338a85 [netgroupman] Remove NetGroupManager::GetAsmap() (John Newbery) 4709fc2019e27e74be02dc5fc123b9f6f46d7990 [netgroupman] Move asmap checksum calculation to NetGroupManager (John Newbery) 1b978a7e8c71dcc1501705022e66f6779c8c4528 [netgroupman] Move GetMappedAS() and GetGroup() logic to NetGroupManager (John Newbery) ddb4101e6377a998b7c598bf52217b47698ddec9 [net] Only use public CNetAddr functions and data in GetMappedAS() and GetGroup() (John Newbery) 6b2268162e96bc4fe1a3ebad454996b1d3d4615c [netgroupman] Add GetMappedAS() and GetGroup() (John Newbery) 19431560e3e1124979c60f39eca9429c4a0df29f [net] Move asmap into NetGroupManager (John Newbery) 17c24d458042229e00dd4e0b75a32e593be29564 [init] Add netgroupman to node.context (John Newbery) 9b3836710b8160d212aacd56154938e5bb4b26b7 [build] Add netgroup.cpp|h (John Newbery) Pull request description: The asmap data is currently owned by addrman, but is used by both addrman and connman. #22791 made the data const and private (so that it can't be updated by other components), but it is still passed out of addrman as a reference to const, and used by `CNetAddress` to calculate the group and AS of the net address. This RFC PR proposes to move all asmap data and logic into a new `NetGroupManager` component. This is initialized at startup, and the client components addrman and connman simply call `NetGroupManager::GetGroup(const CAddress&)` and `NetGroupManager::GetMappedAS(const CAddress&)` to get the net group and AS of an address. ACKs for top commit: mzumsande: Code Review ACK 36f814c0e84d009c0e0aa26981a20ac4cf338a85 jnewbery: CI failure seems spurious. I rebased onto latest master to trigger a new CI run, but whilst I was doing that, mzumsande ACKed https://github.com/bitcoin/bitcoin/commit/36f814c0e84d009c0e0aa26981a20ac4cf338a85, so I've reverted to that. dergoegge: Code review ACK 36f814c0e84d009c0e0aa26981a20ac4cf338a85 Tree-SHA512: 244a89cdfd720d8cce679eae5b7951e1b46b37835fccb6bdfa362856761bb110e79e263a6eeee8246140890f3bee2850e9baa7bc14a388a588e0e29b9d275175
2022-04-20[net] Move asmap into NetGroupManagerJohn Newbery
2022-04-20Merge bitcoin/bitcoin#24909: refactor: Move and rename `pindexBestHeader`, ↡MarcoFalke
`fHavePruned` f0a2fb3c5dbf3c4bec7faf934baff3e723734b3f scripted-diff: Rename pindexBestHeader, fHavePruned (Carl Dong) a4014021258319941716d6338c18667462a06280 Clear fHavePruned in BlockManager::Unload() (Carl Dong) 3308ecd3fc254ee4ef9f803c09f00ba4dc968520 move-mostly: Make fHavePruned a BlockMan member (Carl Dong) c96524113c48553c4bbad63077a25494eca8159e Clear pindexBestHeader in ChainstateManager::Unload() (Carl Dong) 73eedaaacc3b5f2dd791997109f2f5312a894336 style-only: Miscellaneous whitespace changes (Carl Dong) 0d567daf23c9fcb2d95b38913ee45a8b0ba3b027 move-mostly: Make pindexBestHeader a ChainMan member (Carl Dong) 5d670173a32ccdcb25d3a6bf97317f0ac774e0ed validation: Load pindexBestHeader in ChainMan (Carl Dong) Pull request description: Split off from #22564 per Marco's suggestion: https://github.com/bitcoin/bitcoin/pull/22564#issuecomment-1100011503 This is basically the move-mostly parts of #22564. The overall intent is to move mutable globals manually reset by `::UnloadBlockIndex` into appropriate structs such that they are cleared at the appropriate times. Please read #22564's description for more rationale. In summary , this PR moves: 1. `pindexBestHeader` -> `ChainstateManager::m_best_header` 2. `fHavePruned` -> `BlockManager::m_have_pruned` ACKs for top commit: ajtowns: ACK f0a2fb3c5dbf3c4bec7faf934baff3e723734b3f -- code review only MarcoFalke: kirby ACK f0a2fb3c5dbf3c4bec7faf934baff3e723734b3f πŸ˜‹ Tree-SHA512: 8d161701af81af1ff42da1b22a6bef2f8626e8642146bc9c3b27f3a7cd24f4d691910a2392b188ae058fec0611a17304dd73f60da695f53832d327f73d2fc963
2022-04-19scripted-diff: Rename pindexBestHeader, fHavePrunedCarl Dong
...to m_best_header and m_have_pruned -BEGIN VERIFY SCRIPT- find_regex="\bpindexBestHeader\b" \ && git grep -l -E "$find_regex" -- src \ | xargs sed -i -E "s@$find_regex@m_best_header@g" find_regex="\bfHavePruned\b" \ && git grep -l -E "$find_regex" -- src \ | xargs sed -i -E "s@$find_regex@m_have_pruned@g" -END VERIFY SCRIPT-
2022-04-19move-mostly: Make fHavePruned a BlockMan memberCarl Dong
[META] In the next commit, we move the clearing of fHavePruned to BlockManager::Unload()
2022-04-19move-mostly: Make pindexBestHeader a ChainMan memberCarl Dong
[META] In the next commit, we move the clearing of pindexBestHeader to ChainstateManager::Unload()
2022-04-19[init] Add netgroupman to node.contextJohn Newbery
This is constructed before addrman and connman, and destructed afterwards. netgroupman does not currently do anything, but will have functionality added in future commits.
2022-04-12init: Prevent -noproxy and -proxy=0 settings from interacting with other ↡Ryan Ofsky
settings Prevent -noproxy and -proxy=0 settings from interacting with -listen, -upnp, and -natpmp settings. These settings started being handled inconsistently in the `AppInitMain` and `InitParameterInteraction` functions starting in commit baf05075fae2cc2625a2a74b35cc66902f3cbfa3 from #6272: https://github.com/bitcoin/bitcoin/blob/baf05075fae2cc2625a2a74b35cc66902f3cbfa3/src/init.cpp#L990-L991 https://github.com/bitcoin/bitcoin/blob/baf05075fae2cc2625a2a74b35cc66902f3cbfa3/src/init.cpp#L687 This commit changes both functions to handle proxy arguments the same way so there are not side effects from specifying a proxy=0 setting.
2022-04-06doc: Add note that -reindex will rebuild optional indexesMartin Zumsande
2022-04-06Merge bitcoin/bitcoin#24758: Disable the syscall sandbox for bitcoin-qt and ↡laanwj
remove gui-related syscalls fabdf9f870a4c07cb3548c3b385438f02179ea88 Remove gui-only syscalls (MarcoFalke) fa0c2aa826282fe40d2ce7becb4eb6d4814447a3 init: Disable syscall sandbox in the bitcoin-qt process (MarcoFalke) Pull request description: It is basically impossible (and a bit out of scope) for us to maintain a sandbox for the qt library. I am not sure if it is possible to only sandbox a few threads in a process, but I doubt this will add no practical benefit anyway, so I am disabling the sandbox for the whole bitcoin-qt process. See also https://github.com/bitcoin/bitcoin/pull/24690#issuecomment-1084372400 ACKs for top commit: laanwj: Code review ACK fabdf9f870a4c07cb3548c3b385438f02179ea88 Tree-SHA512: 944ded03ee25f7dfd0bfeea9c3f97f575f2d470aa03b387b07f3e3bec5cb886e4aaa17e4a9fb359d3e670e6da69adc9111673d13e6561ec55b3161bb67dfe760
2022-04-05Merge bitcoin/bitcoin#24236: Remove utxo db upgrade codelaanwj
fa9112aac07dc371bfda437d40eb1b841f36f392 Remove utxo db upgrade code (MarcoFalke) Pull request description: It is not possible to upgrade Bitcoin Core pre-segwit (pre-0.13.1) to a recent version without a full IBD from scratch after commit 19a56d1519fb493c3e1bd5cad55360b6b80fa52b (released in version 22.0). Any Bitcoin Core version with the new database format after commit 1088b02f0ccd7358d2b7076bb9e122d59d502d02 (released in version 0.15), can upgrade to any version that is supported as of today. This leaves the versions 0.13.1-0.14.x. Even though those versions are unsupported, some users with an existing datadir may want to upgrade to a recent version. However, it seems reasonable to simply ask them to `-reindex` to run a full IBD from scratch. This allows us to remove the utxo db upgrade code. ACKs for top commit: Sjors: re-ACK fa9112aac07dc371bfda437d40eb1b841f36f392 laanwj: Code review ACK fa9112aac07dc371bfda437d40eb1b841f36f392 Tree-SHA512: 4243bb35df9ac4892f9fad30fe486d338745952bcff4160bcb0937c772d57b13b800647da14695e21e3655e85ee0d95fa3dc7789ee309d59ad84f422297fecb8