aboutsummaryrefslogtreecommitdiff
path: root/src/init.cpp
AgeCommit message (Collapse)Author
2024-09-11assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed. Github-Pull: bitcoin/bitcoin#30807 Rebased-From: 6d5812e5c852c233bd7ead2ceef051f8567619ed
2024-08-07Merge bitcoin/bitcoin#29775: Testnet4 including PoW difficulty adjustment fixAva Chow
6bfa26048dbafb91e9ca63ea8d3960271e798098 testnet: Add timewarp attack prevention for Testnet4 (Fabian Jahr) 0100907ca168c53e8fe044bdda396f308825162c testnet: Add Testnet4 difficulty adjustment rules fix (Fabian Jahr) 74a04f9e7ad6a16988149cc3438b9ce13c91cdb9 testnet: Introduce Testnet4 (Fabian Jahr) Pull request description: To supplement the [ongoing conceptual discussion about a testnet reset](https://groups.google.com/g/bitcoindev/c/9bL00vRj7OU/m/9yCPo3uUBwAJ) I have drafted a move to v4 including a fix to the difficulty adjustment mechanism, which was part of the motivation that started the discussion. Conceptual considerations: - The conceptual discussion about doing a testnet4 or softforking the fix into testnet3 is outside of the scope of this PR and I would ask reviewers to contribute their opinions on this on the ML instead. However, I am happy to adapt this PR to a softfork change on testnet3 if there is consensus for that instead. - The difficulty adjustment fix suggested here touches the `CalculateNextWorkRequired` function and uses the same logic used in `GetNextWorkRequired` to find the last previous block that was not mined with difficulty 1 under the exceptionf. An alternative fix briefly mentioned on the mailing list by Jameson Lopp would be to "restrict the special testnet minimum difficulty rule so that it can't be triggered on the block right before a difficulty retarget". That would also fix the issue but I find my suggestion here a bit more elegant. ACKs for top commit: jsarenik: tACK 6bfa26048dba achow101: ACK 6bfa26048dbafb91e9ca63ea8d3960271e798098 murchandamus: tACK 6bfa26048dbafb91e9ca63ea8d3960271e798098 Tree-SHA512: 0b8b69a621406a944da5be551b863d065358ba94d85dd3b80d83c412660e230ee93b27316081fbee9b4851cc4ff8585db64c7dfa26cb5148ac835663f2712c3d
2024-08-07add deprecation warning for mempoolfullrbfglozow
2024-08-06testnet: Introduce Testnet4Fabian Jahr
2024-07-26Add -blocksxor boolean optionMarcoFalke
2024-07-16Merge bitcoin/bitcoin#22729: Make it possible to disable Tor binds and abort ↵Ava Chow
startup on bind failure bca346a97056748f1e3fb899f336d56d9fd45a64 net: require P2P binds to succeed (Vasil Dimov) af552534ab83c572d3bc3f93ccaee5c1961ccab5 net: report an error if unable to bind on the Tor listening addr:port (Vasil Dimov) 9a7e5f4d68dadc64a675f32d1e91199d6b1aa095 net: don't extra bind for Tor if binds are restricted (Vasil Dimov) Pull request description: Make it possible to disable the Tor binding on `127.0.0.1:8334` and stop startup if any P2P bind fails instead of "if all P2P binds fail". Fixes https://github.com/bitcoin/bitcoin/issues/22726 Fixes https://github.com/bitcoin/bitcoin/issues/22727 ACKs for top commit: achow101: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 cbergqvist: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 pinheadmz: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 Tree-SHA512: fabef89a957191eea4f3e3b6109d2b8389f27ecc74440a920b0c10f31fff00a85bcfd1eb3c91826c7169c618f4de8a8d0a260e2caf40fd854f07ea9a980d8603
2024-07-12init: change shutdown order of load block thread and schedulerMartin Zumsande
This avoids situations during a reindex in which shutdown doesn't finish since SyncWithValidationInterfaceQueue is called by the load block thread when the scheduler is already stopped.
2024-07-08Merge bitcoin/bitcoin#30141: kernel: De-globalize validation cachesRyan Ofsky
606a7ab862470413ced400aa68a94fd37c8ad3d3 kernel: De-globalize signature cache (TheCharlatan) 66d74bfc45ae0f743084475ac3bbfb4355bb6ec2 Expose CSignatureCache class in header (TheCharlatan) 021d38822c0e6a1b9497bcb20401c5c37e1bb84d kernel: De-globalize script execution cache hasher (TheCharlatan) 13a3661aba95b54b822c99ecbb695b14a22536d2 kernel: De-globalize script execution cache (TheCharlatan) ab14d1d6a4a8ef5fe5013150e6c5ebcb5f5e4ea9 validation: Don't error if maxsigcachesize exceeds uint32::max (TheCharlatan) Pull request description: The validation caches are currently setup independently from where the rest of the validation code is initialized. This makes their ownership semantics unclear. There is also no clear enforcement on when and in what order they need to be initialized. The caches are always initialized in the `BasicTestingSetup` although a number of tests don't actually need them. Solve this by moving the caches from global scope into the `ChainstateManager` class. This simplifies the usage of the kernel library by no longer requiring manual setup of the caches prior to using the `ChainstateManager`. Tests that need to access the caches can instantiate them independently. --- This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: stickies-v: re-ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3 glozow: reACK 606a7ab ryanofsky: Code review ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3. Just small formatting, include, and static_assert changes since last review. Tree-SHA512: e7f3ee41406e3b233832bb67dc3a63c4203b5367e5daeed383df9cb590f227fcc62eae31311029c077d5e81b273a37a88a364db3dee2efe91bb3b9c9ddc8a42e
2024-07-05kernel: De-globalize signature cacheTheCharlatan
Move its ownership to the ChainstateManager class. Next to simplifying usage of the kernel library by no longer requiring manual setup of the cache prior to using validation code, it also slims down the amount of memory allocated by BasicTestingSetup. Use this opportunity to make SignatureCache RAII styled Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-04kernel: De-globalize script execution cacheTheCharlatan
Move its ownership to the ChainstateManager class. Next to simplifying usage of the kernel library by no longer requiring manual setup of the cache prior to using validation code, it also slims down the amount of memory allocated by BasicTestingSetup.
2024-07-04validation: Don't error if maxsigcachesize exceeds uint32::maxTheCharlatan
Instead clamp it to uint32::max if it exceeds it. Co-authored-by: Anthony Towns <aj@erisian.com.au>
2024-07-04Merge bitcoin/bitcoin#29625: Several randomness improvementsmerge-script
ce8094246ee95232e9d84f7e37f3c0a43ef587ce random: replace construct/assign with explicit Reseed() (Pieter Wuille) 2ae392d561ecfdf81855e6df6b9ad3d8843cdfa2 random: use LogError for init failure (Pieter Wuille) 97e16f57042cab07e5e73f6bed19feec2006e4f7 tests: make fuzz tests (mostly) deterministic with fixed seed (Pieter Wuille) 2c91330dd68064e402e8eceea3df9474bb7afd48 random: cleanup order, comments, static (Pieter Wuille) 8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c net, net_processing: use existing RNG objects more (Pieter Wuille) d5fcbe966bc501db8bf6a3809633f0b82e6ae547 random: improve precision of MakeExponentiallyDistributed (Pieter Wuille) cfb0dfe2cf0b46f3ea9e62992ade989860f086c8 random: convert GetExponentialRand into rand_exp_duration (Pieter Wuille) 4eaa239dc3e189369d59144b524cb2808cbef8c3 random: convert GetRand{Micros,Millis} into randrange (Pieter Wuille) 82de1b80d95fc9447e64c098dcadb6b8a2f1f2ee net: use GetRandMicros for cache expiration (Pieter Wuille) ddc184d999d7e1a87efaf6bcb222186f0dcd87ec random: get rid of GetRand by inlining (Pieter Wuille) e2d1f84858485650ff743753ffa5c679f210a992 random: make GetRand() support entire range (incl. max) (Pieter Wuille) 810cdf6b4e12a1fdace7998d75b4daf8b67d7028 tests: overhaul deterministic test randomness (Pieter Wuille) 6cfdc5b104caf9952393f9dac2a36539d964077f random: convert XoRoShiRo128PlusPlus into full RNG (Pieter Wuille) 8cc2f45065fc1864f879248d1e1444588e27076b random: move XoRoShiRo128PlusPlus into random module (Pieter Wuille) 8f5ac0d0b608bdf396d8f2d758a792f869c2cd2a xoroshiro128plusplus: drop comment about nonexisting copy() (Pieter Wuille) 8924f5120f66269c04633167def01f82c74ea730 random: modernize XoRoShiRo128PlusPlus a bit (Pieter Wuille) ddb7d26cfd96c1f626def4755e0e1b5aaac94d3e random: add RandomMixin::randbits with compile-known bits (Pieter Wuille) 21ce9d8658fed0d3e4552e8b02a6902cb31c572e random: Improve RandomMixin::randbits (Pieter Wuille) 9b14d3d2da05f74ffb6a2ac20b7d9efefbe29634 random: refactor: move rand* utilities to RandomMixin (Pieter Wuille) 40dd86fc3b60d7a67a9720a84a685f16e3f05b06 random: use BasicByte concept in randbytes (Pieter Wuille) 27cefc7fd6a6a159779f572f4c3a06170f955ed8 random: add a few noexcepts to FastRandomContext (Pieter Wuille) b3b382dde202ad508baf553817c5b38fdd2d4a0c random: move rand256() and randbytes() to .h file (Pieter Wuille) 493a2e024e845e623e202e3eefe1cc2010e9b514 random: write rand256() in function of fillrand() (Pieter Wuille) Pull request description: This PR contains a number of vaguely-related improvements to the random module. The specific changes and more detailed rationale is in the commit messages, but the highlights are: * `XoRoShiRo128PlusPlus` (previously a test-only RNG) moves to random.h and becomes `InsecureRandomContext`, which is even faster than `FastRandomContext` but non-cryptographic. It also gets all helper randomness functions (`randrange`, `fillrand`, ...), making it a lot more succinct to use. * During tests, **all** randomness is made deterministic (except for `GetStrongRandBytes`) but non-repeating (like `GetRand()` used to be when `g_mock_deterministic_tests` was used), either fixed, or from a random seed (overridden by env var). * Several infrequently used top-level functions (`GetRandMillis`, `GetRandMicros`, `GetExponentialRand`) are converted into member functions of `FastRandomContext` (and `InsecureRandomContext`). * `GetRand<T>()` (without argument) can now return the maximum value of the type (previously e.g. `GetRand<uint32_t>()` would never return 0xffffffff). ACKs for top commit: achow101: ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce maflcko: re-ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce 🐈 hodlinator: ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce dergoegge: utACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce Tree-SHA512: 79bc0cbafaf27e95012c1ce2947a8ca6f9a3c78af5f1f16e69354b6fc9b987a28858adf4cd356dc5baf21163e9af8dcc24e70f8d7173be870e8a3ddcdd47c02c
2024-07-02Merge bitcoin/bitcoin#30324: optimization: Moved repeated `-printpriority` ↵Ava Chow
fetching out of AddToBlock 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 Moved the repeated -printpriority fetching out of AddToBlock (Lőrinc) Pull request description: `AddToBlock` was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time. <img src="https://github.com/bitcoin/bitcoin/assets/1841944/6fd89647-7b6c-4f44-bd04-98d16cd2a938"> This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a small speed increase for many iterations. > ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=10000 before: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 156,460.15 | 6,391.40 | 0.1% | 11.03 | `AssembleBlock` after: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 149,289.55 | 6,698.39 | 0.3% | 10.97 | `AssembleBlock` --- The slight speedup shows up in CI as well: <img src="https://github.com/bitcoin/bitcoin/assets/1841944/3be779c9-2dce-4a96-ae5f-cab5435bd72f"> ACKs for top commit: maflcko: ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 achow101: ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 tdb3: re ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6 furszy: utACK 323ce303086 Tree-SHA512: c2a0aab429646453ad0470956529f1cac8c38778c4c53f82c92c6cbaaaeb69f3d3603c0014ff097844b151e9da7caa2371a4676244caea96527cb540e66825a3
2024-07-02net: don't extra bind for Tor if binds are restrictedVasil Dimov
If only `-bind=addr:port` is given (without `-bind=...=onion`) then we would bind to `addr:port` _and_ to `127.0.0.1:8334` in addition which may be unexpected, assuming the semantic of `-bind=addr:port` is "bind _only_ to `addr:port`". Change the above to not do the additional bind: if only `-bind=addr:port` is given (without `-bind=...=onion`) then bind to `addr:port` (only). If we are creating a Tor hidden service then use `addr:port` as target (same behavior as before https://github.com/bitcoin/bitcoin/pull/19991). This allows disabling binding on the onion port. Fixes https://github.com/bitcoin/bitcoin/issues/22726
2024-07-02Merge bitcoin/bitcoin#30344: kernel: remove mempool_persistglozow
f1478c05458562a9bef5c2ba43959d758e7b4745 mempool: move LoadMempool/DumpMempool to node (Cory Fields) 6d242ff1e9ca02fd8f5cb3ffe82dfb48a52366cc kernel: remove mempool_persist.cpp (Cory Fields) Pull request description: DumpMempool/LoadMempool are not necessary for the kernel. Noticed while working on instantiated logging. I suppose these could have been left in on purpose, but I'm assuming it was probably just an oversight. ACKs for top commit: TheCharlatan: Re-ACK f1478c05458562a9bef5c2ba43959d758e7b4745 glozow: ACK f1478c0545 stickies-v: ACK f1478c05458562a9bef5c2ba43959d758e7b4745 Tree-SHA512: 5825da0cf2e67470524eb6ebe397eb90755a368469a25f184df99ab935b3eb6d89eb802b41a6c3661e869bba3bbfa8ba9d95281bc75ebbf790ec5d9d1f79c66f
2024-07-01random: get rid of GetRand by inliningPieter Wuille
2024-06-30Moved the repeated -printpriority fetching out of AddToBlockLőrinc
AddToBlock was called repeatedly from `addPackageTxs` where the constant value of `printpriority` is recalculated every time. Since its behavior was changed in 400b151, I've named the variable accordingly. This showed up during profiling of AssembleBlock, fetching it once in the constructor results in a measurable speed increase for many iterations. > ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=1000 before: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 155,558.97 | 6,428.43 | 0.1% | 1.10 | `AssembleBlock` after: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 148,083.68 | 6,752.94 | 0.1% | 1.10 | `AssembleBlock` Co-authored-by: furszy <mfurszy@protonmail.com>
2024-06-28scripted-diff: Log parameter interaction not thriceMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i 's/LogPrintf("%s: \(parameter interaction: .*\)", __func__/LogInfo("\1"/g' ./src/init.cpp -END VERIFY SCRIPT-
2024-06-28doc: Fix outdated dev comment about loggingMarcoFalke
2024-06-27Merge bitcoin/bitcoin#28167: init: Add option for rpccookie permissions ↵Ryan Ofsky
(replace 26088) 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe doc: detail -rpccookieperms option (willcl-ark) d2afa2690cceb0012b2aa1960e1cfa497f3103fa test: add rpccookieperms test (willcl-ark) f467aede78533dac60a118e1566138d65522c213 init: add option for rpccookie permissions (willcl-ark) 7df03f1a923e239cea8c9b0d603a9eb00863a40c util: add perm string helper functions (willcl-ark) Pull request description: This PR picks up #26088 by aureleoules which adds a bitcoind launch option `-rpccookieperms` to set the file permissions of the cookie generated by bitcoin core. Example usage to make the generated cookie group-readable: `./src/bitcoind -rpccookieperms=group`. Accepted values for `-rpccookieperms` are `[owner|group|all]`. We let `fs::perms` handle platform-specific permissions changes. ACKs for top commit: achow101: ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe ryanofsky: Code review ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe. Main change since last review is no longer throwing a skip exception in the rpc test on windows, so other checks can run after it, and overall test result is passing, not skipped. Also were clarifying renames and documentation improvements. tdb3: cr ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe Tree-SHA512: e800d59a44aca10e1c58ca69bf3fdde9f6ccf5eab4b7b962645af6d6bc0cfa3a357701e409c8c60d8d7744fcd33a91e77ada11790aa88cd7811ef60fab86ab11
2024-06-27init: add option for rpccookie permissionswillcl-ark
Add a bitcoind launch option `-rpccookieperms` to configure the file permissions of the cookie on Unix systems.
2024-06-26mempool: move LoadMempool/DumpMempool to nodeCory Fields
2024-06-24Merge bitcoin/bitcoin#30200: Introduce Mining interfaceRyan Ofsky
a9716c53f05082d6d89ebea51a46d4404efb12d7 rpc: call IsInitialBlockDownload via miner interface (Sjors Provoost) dda0b0834faf7be7e8938bf63e7bb01cd54a416a rpc: minize getTipHash() calls in gbt (Sjors Provoost) 7b4d3249ced93ec5986500e43b324005ed89502f rpc: call processNewBlock via miner interface (Sjors Provoost) 9e228351e761d8d24413bbc4ac1610b4f3dec2bf rpc: getTransactionsUpdated via miner interface (Sjors Provoost) 64ebb0f97178687517c2060bf6b9931064607888 Always pass options to BlockAssembler constructor (Sjors Provoost) 4bf2e361da1964f7c278b4939967a0e5afde20b0 rpc: call CreateNewBlock via miner interface (Sjors Provoost) 404b01c436122b951e9e06ed26d79dba4651685e rpc: getblocktemplate getTipHash() via Miner interface (Sjors Provoost) d8a3496b5ad27bea4c79ea0344f595cc1b95f0d3 rpc: call TestBlockValidity via miner interface (Sjors Provoost) 8ecb6816781c7c7f423b501cbb2de3abd7250119 Introduce Mining interface (Sjors Provoost) Pull request description: Introduce a `Mining` interface for the `getblocktemplate`, `generateblock` and other mining RPCs to use now, and for Stratum v2 to use later. Suggested here: https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2108528652 The selection of methods added to the interface is mostly based on what the Template Provider in #29432 uses. It could be expanded further so that `rpc/mining.cpp` no longer needs `EnsureMemPool` and `EnsureChainman`. This PR should be a pure refactor. ACKs for top commit: tdb3: re ACK a9716c53f05082d6d89ebea51a46d4404efb12d7 itornaza: Code review and std-tests ACK a9716c53f05082d6d89ebea51a46d4404efb12d7 ryanofsky: Code review ACK a9716c53f05082d6d89ebea51a46d4404efb12d7 with one minor suggestion in case you update. Only changes since last review were other small changes to the interface. Tree-SHA512: cf97f87d6e9ed89da3835a0730da3b24a7b14c8605ea221149103a5915e79598cf082a95f2bc88e33f1c450e3d4aad88aed1163a29195acca88bcace055af724
2024-06-21refactor: use #ifdef HAVE_SOCKADDR_UNfanquake
```bash init.cpp:526:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 526 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ init.cpp:541:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 541 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ init.cpp:1318:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 1318 | #if HAVE_SOCKADDR_UN ``` ``` netbase.cpp:26:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 26 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:221:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 221 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:496:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 496 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:531:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 531 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:639:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 639 | #if HAVE_SOCKADDR_UN ```
2024-06-21zmq: use #ifdef ENABLE_ZMQfanquake
2024-06-18Introduce Mining interfaceSjors Provoost
Start out with a single method isTestChain() that's used by the getblocktemplate RPC.
2024-06-17Merge bitcoin/bitcoin#30058: Encapsulate warnings in generalized ↵Ava Chow
node::Warnings and remove globals 260f8da71a35232d859d7705861fc1a88bfbbe81 refactor: remove warnings globals (stickies-v) 9c4b0b7ce459765fa1a63b410c3423b90f0d2a5f node: update uiInterface whenever warnings updated (stickies-v) b071ad9770b7ae7fc718dcbfdc8f62dffbf6cfee introduce and use the generalized `node::Warnings` interface (stickies-v) 20e616f86444d00712ac7eb840666e2b0378af4a move-only: move warnings from common to node (stickies-v) bed29c481aebeb2b0160450c63c03cc68fb89bc6 refactor: remove unnecessary AppendWarning helper function (stickies-v) Pull request description: This PR: - moves warnings from common to the node library and into the node namespace (as suggested in https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570069541) - generalizes the warnings interface to `Warnings::Set()` and `Warnings::Unset()` methods, instead of having a separate function and globals for each warning. As a result, this simplifies the `kernel::Notifications` interface. - removes warnings.cpp from the kernel library - removes warning globals - adds testing for the warning logic Behaviour change introduced: - the `-alertnotify` command is executed for all `KernelNotifications::warningSet` calls, which now also covers the `LARGE_WORK_INVALID_CHAIN` warning - the GUI is updated automatically whenever a warning is (un)set, covering some code paths where it previously wouldn't be, e.g. when `node::AbortNode()` is called, or for the `LARGE_WORK_INVALID_CHAIN` warning Some discussion points: - ~is `const std::string& id` the best way to refer to warnings? Enums are an obvious alternative, but since we need to define warnings across libraries, strings seem like a straightforward solution.~ _edit: updated approach to use `node::Warning` and `kernel::Warning` enums._ ACKs for top commit: achow101: ACK 260f8da71a35232d859d7705861fc1a88bfbbe81 ryanofsky: Code review ACK 260f8da71a35232d859d7705861fc1a88bfbbe81. Only change since last review was rebasing TheCharlatan: Re-ACK 260f8da71a35232d859d7705861fc1a88bfbbe81 Tree-SHA512: a3fcedaee0d3ad64e9c111aeb30665162f98e0e72acd6a70b76ff2ddf4f0a34da4f97ce353c322a1668ca6ee4d8a81cc6e6d170c5bbeb7a43cffdaf66646b588
2024-06-14Merge bitcoin/bitcoin#30255: log: use error level for critical log messagesAva Chow
fae3a1f0065064d80ab4c0375a9eaeb666c5dd55 log: use error level for critical log messages (MarcoFalke) Pull request description: This picks up the first commit from https://github.com/bitcoin/bitcoin/pull/29231, but extends it to also cover cases that were missed in it. As per https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#logging, LogError should be used for severe problems that require the node to shut down. ACKs for top commit: stickies-v: re-ACK fae3a1f0065064d80ab4c0375a9eaeb666c5dd55, I'm ~0 on the latest force push as `user_error` was already logged at the right level through `GetNotifications().fatalError(user_error);` so I'd be in favour of deduplicating/cleaning up this logging logic but can be done in follow-up. kevkevinpal: ACK [fae3a1f](https://github.com/bitcoin/bitcoin/pull/30255/commits/fae3a1f0065064d80ab4c0375a9eaeb666c5dd55) achow101: ACK fae3a1f0065064d80ab4c0375a9eaeb666c5dd55 Tree-SHA512: 3f99fd25d5a204d570a42d8fb2b450439aad7685692f9594cc813d97253c4df172a6ff3cf818959bfcf25dfcf8ee9a9c9ccc6028fcfcecdb47591e18c77ef246
2024-06-13refactor: remove warnings globalsstickies-v
2024-06-12Merge bitcoin/bitcoin#29015: kernel: Streamline util libraryAva Chow
c7376babd19d0c858fef93ebd58338abd530c1f4 doc: Clarify distinction between util and common libraries in libraries.md (Ryan Ofsky) 4f74c59334d496f28e1a5c0d84c412f9020b366f util: Move util/string.h functions to util namespace (Ryan Ofsky) 4d05d3f3b42a41525aa6ec44b90f543dfab53ecf util: add TransactionError includes and namespace declarations (Ryan Ofsky) 680eafdc74021c1e0893c3a62404e607fd4724f5 util: move fees.h and error.h to common/messages.h (Ryan Ofsky) 02e62c6c9af4beabaeea58fb1ea3ad0dc5094678 common: Add PSBTError enum (Ryan Ofsky) 0d44c44ae33434f366229c612d6edeedf7658963 util: move error.h TransactionError enum to node/types.h (Ryan Ofsky) 9bcce2608dd2515dc35a0f0866abc9d43903c795 util: move spanparsing.h to script/parsing.h (Ryan Ofsky) 6dd2ad47922694d2ab84bad4dac9dd442c5df617 util: move spanparsing.h Split functions to string.h (Ryan Ofsky) 23cc8ddff472d259605d7790ba98a1900e77efab util: move HexStr and HexDigit from util to crypto (TheCharlatan) 6861f954f8ff42c87ad638037adae86a5bd89600 util: move util/message to common/signmessage (Ryan Ofsky) cc5f29fbea15d33e4d1aa95591253c6b86953fe7 build: move memory_cleanse from util to crypto (Ryan Ofsky) 5b9309420cc9721a0d5745b6ad3166a4bdbd1508 build: move chainparamsbase from util to common (Ryan Ofsky) ffa27af24da81a97d6c4912ae0e10bc5b6f17f69 test: Add check-deps.sh script to check for unexpected library dependencies (Ryan Ofsky) Pull request description: Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically: - Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity. - Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing. - Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of the util library. Motivation for this change is that the util library is a dependency of the kernel, and we should remove functionality from util that shouldn't be called by kernel code or kernel applications. These changes should also improve code organization and make functions easier to discover. Some of these same moves are (or were) part of #28690, but did not help with code organization, or made it worse, so it is better to move them and clean them up in the same PR so code only has to change one time. ACKs for top commit: achow101: ACK c7376babd19d0c858fef93ebd58338abd530c1f4 TheCharlatan: Re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4 hebasto: re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4. Tree-SHA512: 5bcef16c1255463b1b69270548711e7ff78ca0dd34e300b95e3ca1ce52ceb34f83d9ddb2839e83800ba36b200de30396e504bbb04fa02c6d0c24a16d06ae523d
2024-06-11Merge bitcoin/bitcoin#28339: validation: improve performance of CheckBlockIndexAva Chow
5bc2077e8f592442b089affdf0b5795fbc053bb8 validation: allow to specify frequency for -checkblockindex (Martin Zumsande) d5a631b9597e5029a5048d9b8ad84ea4536bbac0 validation: improve performance of CheckBlockIndex (Martin Zumsande) 32c80413fdb063199f3bee719c4651bd63f05fce bench: add benchmark for checkblockindex (Martin Zumsande) Pull request description: `CheckBlockIndex() ` are consistency checks that are currently enabled by default on regtest. The function is rather slow, which is annoying if you * attempt to run it on other networks, especially if not fully synced * want to generate a long chain on regtest and see block generation slow down because you forgot to disable `-checkblockindex` or don't know it existed. One reason why it's slow is that in order to be able to traverse the block tree depth-first from genesis, it inserts pointers to all block indices into a `std::multimap` - for which inserts and lookups become slow once there are hundred thousands of entries. However, typically the block index is mostly chain-like with just a few forks so a multimap isn't really needed for the most part. This PR suggests to store the block indices of the chain ending in the best header in a vector instead, and store only the rest of the indices in a multimap. This does not change the actual consistency checks that are being performed for each index, just the way the block index tree is stored and traversed. This adds a bit of complication to make sure each block is visited (note that there are asserts that check it), making sure that the two containers are traversed correctly, but it speeds up the function considerably: On master, a single invocation of `CheckBlockIndex` takes ~1.4s on mainnet for me (4.9s on testnet which has >2.4 million blocks). With this branch, the runtime goes down to ~0.27s (0.85s on testnet).This is a speedup by a factor ~5. ACKs for top commit: achow101: ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8 furszy: ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8 ryanofsky: Code review ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8. Just added suggested assert and simplification since last review Tree-SHA512: 6b9c3e3e5069d6152b45a09040f962380d114851ff0f9ff1771cf8cad7bb4fa0ba25cd787ceaa3dfa5241fb249748e2ee6987af0ccb24b786a5301b2836f8487
2024-06-11Merge bitcoin/bitcoin#28830: [refactor] Check CTxMemPool options in ctorAva Chow
09ef322acc0a88a9e119f74923399598984c68f6 [[refactor]] Check CTxMemPool options in constructor (TheCharlatan) Pull request description: The tests should run the same checks on the mempool options that the init code also applies. The downside to this patch is that the log line may now be printed more than once in the for loop. This was originally noticed here https://github.com/bitcoin/bitcoin/pull/25290#discussion_r900272797. ACKs for top commit: stickies-v: re-ACK 09ef322acc0a88a9e119f74923399598984c68f6 . Fixed unreachable assert and updated docstring, and also added an exception for "-maxmempool must be at least " in the `tx_pool` fuzz test, which makes sense when looking at how the mempool options are constructed in `SetMempoolConstraints`. achow101: ACK 09ef322acc0a88a9e119f74923399598984c68f6 ryanofsky: Code review ACK 09ef322acc0a88a9e119f74923399598984c68f6. Just fuzz test error checking fix and updated comment since last review Tree-SHA512: eb3361411c2db70be17f912e3b14d9cb9c60fb0697a1eded952c3b7e8675b7d783780d45c52e091931d1d80fe0f0280cee98dd57a3100def13af20259d9d1b9e
2024-06-10log: use error level for critical log messagesMarcoFalke
As per doc/developer-notes#logging, LogError should be used for severe problems that require the node to shut down. Co-Authored-By: stickies-v <stickies-v@protonmail.com>
2024-06-07blockman: Replace m_reindexing with m_blockfiles_indexedRyan Ofsky
This is a just a mechanical change, renaming and inverting the meaning of the indexing variable. "m_blockfiles_indexed" is a more straightforward name for this variable because this variable just indicates whether or not <datadir>/blocks/blk?????.dat files have been indexed in the <datadir>/blocks/index LevelDB database. The name "m_reindexing" was more confusing, it could be true even if -reindex was not specified, and false when it was specified. Also, the previous name unnecessarily required thinking about the whole reindexing process just to understand simple checks in validation code about whether blocks were indexed. The motivation for this change is to follow up on previous commits, moving away from having multiple variables called "reindex" internally, and instead naming variables individually after what they do and represent.
2024-06-07indexes: Don't wipe indexes again when already reindexingTheCharlatan
Before this change continuing a reindex without the -reindex flag set would leave the block and coins db intact, but discard the data of the optional indexes. While not a bug per se, wiping the data again is wasteful, both in terms of having to write it again, and potentially leading to longer startup times. When initially running a reindex, both the block index and any further activated indexes are wiped. On an index's Init(), both the best block stored by the index and the chain's tip are null. An index's m_synced member is therefore true. This means that it will process blocks through validation events while the reindex is running. Currently, if the reindex is continued without the user re-specifying the reindex flag, the block index is preserved but further index data is wiped. This leads to the stored best block being null, but the chain tip existing. The m_synced member will be set to false. The index will not process blocks through the validation interface, but instead use the background sync once the reindex is completed. If the index is preserved (this change) after a restart its best block may potentially match the chain tip. The m_synced member will be set to true and the index can process validation events during the rest of the reindex.
2024-06-07kernel: Add less confusing reindex optionsRyan Ofsky
Drop confusing kernel options: BlockManagerOpts::reindex ChainstateLoadOptions::reindex ChainstateLoadOptions::reindex_chainstate Replacing them with more straightforward options: ChainstateLoadOptions::wipe_block_tree_db ChainstateLoadOptions::wipe_chainstate_db Having two options called "reindex" which did slightly different things was needlessly confusing (one option wiped the block tree database, and the other caused block files to be rescanned). Also the previous set of options did not allow rebuilding the block database without also rebuilding the chainstate database, when it should be possible to do those independently.
2024-06-07bugfix: Streamline setting reindex optionTheCharlatan
Reverts a bug introduced in b47bd959207e82555f07e028cc2246943d32d4c3 "kernel: De-globalize fReindex". The change leads to a GUI user being prompted to re-index on a chainstate loading failure more than once as well as the node actually not reindexing if the user chooses to. Fix this by setting the reindexing option instead of the atomic, which can be safely re-used to indicate that a reindex should be attempted. The bug specifically is caused by the chainman, and thus the blockman and its m_reindexing atomic being destroyed on every iteration of the for loop. The reindex option for ChainstateLoadOptions is currently also set in a confusing way. By using the reindex atomic, it is not obvious in which scenario it is true or false. The atomic is controlled by both the user passing the -reindex option, the user chosing to reindex if something went wrong during chainstate loading when running the gui, and by reading the reindexing flag from the block tree database in LoadBlockIndexDB. In practice this read is done through the chainstate module's CompleteChainstateInitialization's call to LoadBlockIndex. Since this is only done after the reindex option is set already, it does not have an effect on it. Make this clear by using the reindex option from the blockman opts which is only controlled by the user.
2024-05-17[[refactor]] Check CTxMemPool options in constructorTheCharlatan
This ensures that the tests run the same checks on the mempool options that the init code also applies.
2024-05-16util: Move util/string.h functions to util namespaceRyan Ofsky
There are no changes to behavior. Changes in this commit are all additions, and are easiest to review using "git diff -U0 --word-diff-regex=." options. Motivation for this change is to keep util functions with really generic names like "Split" and "Join" out of the global namespace so it is easier to see where these functions are defined, and so they don't interfere with function overloading, especially since the util library is a dependency of the kernel library and intended to be used with external code.
2024-05-16util: move fees.h and error.h to common/messages.hRyan Ofsky
Move enum and message formatting functions to a common/messages header where they should be more discoverable, and also out of the util library, so they will not be a dependency of the kernel The are no changes in behavior and no changes to the moved code.
2024-05-16kernel: De-globalize fReindexTheCharlatan
fReindex is one of the last remaining globals exposed by the kernel library, so move it into the blockstorage class to reduce the amount of global mutable state and make the kernel library a bit less awkward to use.
2024-05-14Merge bitcoin/bitcoin#30083: kernel: Remove batchpriority from kernel libraryRyan Ofsky
d4b17c7d46ad8e2833ade99d5b4c9741c913e84d kernel: Remove batchpriority from kernel library (TheCharlatan) Pull request description: The current usage of ScheduleBatchPriority is not transparent. Once the thread scheduling is changed, it remains unchanged for the remainder of the thread's lifetime. So move the call from `ImportBlocks` to the init code where it is clearer that its effect lasts for the entire lifetime of the thread. Users of the kernel library might not expect `ImportBlocks` to have an influence on the thread it is called in. Particularly since it is only a compile time option and cannot be controlled at runtime. With this patch users of the kernel library can now freely choose their own scheduling policy. This PR is easier reviewed with `git diff --color-moved-ws=ignore-all-space --color-moved=dimmed-zebra` --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: maflcko: ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d 📭 ryanofsky: Code review ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, just added suggested comment since last review hebasto: ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, I have reviewed the code and it looks OK. Tree-SHA512: cafedecd9affad58ddd7f30f68bba71291ca951bb186ff4b2da04b7f21f0b26e5e3143846d032b9e391bd5ce6c7466b97aa3758d2a85ebd7353eb8b69139641a
2024-05-14kernel: Remove batchpriority from kernel libraryTheCharlatan
The current usage of ScheduleBatchPriority is not transparent. Once the thread scheduling is changed, it remains unchanged for the remainder of the thread's lifetime. So move the call from `ImportBlocks` to the init code where it is clearer that its effect lasts for the entire lifetime of the thread. Users of the kernel library might not expect `ImportBlocks` to have an influence on the thread it is called in. Particularly since it is only a compile time option and cannot be controlled at runtime. With this patch users of the kernel library can now choose their own scheduling policy.
2024-05-13Merge bitcoin/bitcoin#29994: doc: removed help text saying that peers may ↵glozow
not connect automatically 95897ff181c0757e445f0e066a2a590a0a0120d2 doc: removed help text saying that peers may not connect automatically (kevkevin) Pull request description: Introduced in https://github.com/bitcoin/bitcoin/pull/23542 and released in version 23.0 there has been significant time since this change (2 years). This should be removed as it is no longer relevant ACKs for top commit: stickies-v: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2 tdb3: ACK for 95897ff181c0757e445f0e066a2a590a0a0120d2. vasild: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2 jonatack: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2 kristapsk: ACK 95897ff181c0757e445f0e066a2a590a0a0120d2. According to https://bitnodes.io/dashboard/#user-agents stats, most nodes on the network are v23+. Tree-SHA512: 9e35194f8a1e06f1447450af2ea30cdc43722665c2d2e4b7aa9a52afac5c1e83fed744742c836743a555cc180c90f9eebdc6637eba6190010d693eef9c5834f7
2024-05-09kernel: Remove key module from kernel libraryTheCharlatan
The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's `secp256k1_context_sign` global as part of the `kernel::Context` through `ECC_Start`.
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-02refactor: Avoid unused-variable warning in init.cppMarcoFalke
2024-05-01Merge bitcoin/bitcoin#29120: test: Add test case for spending bare multisigmerge-script
e504b1fa1fa4d014b329dea81bfdf1aa55db238f test: Add test case for spending bare multisig (Brandon Odiwuor) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/29113 ACKs for top commit: ajtowns: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f ; LGTM and just checking the 1-of-3 case seems fine maflcko: utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f achow101: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f willcl-ark: reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
2024-05-01scripted-diff: Add IWYU pragma keep to bitcoin-config.h includesMarcoFalke
-BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' ) -END VERIFY SCRIPT-
2024-04-30Merge bitcoin/bitcoin#28016: p2p: gives seednode priority over dnsseed if ↵Ava Chow
both are provided 82f41d76f1c6ad38290917dad5499ffbe6b3974d Added seednode prioritization message to help output (tdb3) 3120a4678ab2a71a381e847688f44068749cfa97 Gives seednode priority over dnsseed if both are provided (Sergi Delgado Segura) Pull request description: This is a follow-up of #27577 If both `seednode` and `dnsseed` are provided, the node will start a race between them in order to fetch data to feed the `addrman`. This PR gives priority to `seednode` over `dnsseed` so if some nodes are provided as seeds, they can be tried before defaulting to the `dnsseeds` ACKs for top commit: davidgumberg: untested reACK https://github.com/bitcoin/bitcoin/commit/82f41d76f1c6ad38290917dad5499ffbe6b3974d itornaza: tested re-ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d achow101: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d cbergqvist: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d Tree-SHA512: 4e39e10a7449af6cd9b8f9f6878f846b94bca11baf89ff2d4fbcd4f28293978a6ed71a3a86cea36d49eca891314c834e32af93f37a09c2cc698a878f84d31c62
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-14 21:42:18 +0000