aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-09-17Merge bitcoin/bitcoin#30438: guix: (explicitly) build Linux GCC with ↵merge-script
`--enable-cet` 89bf11b807252fe5839b5b18742e24568dfe7bbd guix: build Linux GCC with --enable-cet (fanquake) Pull request description: Similar to #29695, and in the same vein of explicitly configuring hardening options in our release toolchain. See https://gcc.gnu.org/install/configure.html: >` --enable-cet` > Enable building target run-time libraries with control-flow instrumentation, see `-fcf-protection option`. When --enable-cet is specified target libraries are configured to add `-fcf-protection` and, if needed, other target specific options to a set of building options. > `--enable-cet=auto` is default. CET is enabled on Linux/x86 if target binutils supports Intel CET instructions and disabled otherwise. In this case, the target libraries are configured to get additional `-fcf-protection` option. ACKs for top commit: TheCharlatan: ACK 89bf11b807252fe5839b5b18742e24568dfe7bbd Tree-SHA512: 772d8529713a31e5db42be4e053582bb9ba6f26079ae136c6bf8303c4992a90d61159dbb0fde7a4b4cb7b4bf5024d5397a78004e6188b36e1c36dd5e5cdc49ad
2024-09-16Merge bitcoin/bitcoin#30884: streams: cache file position within AutoFileAva Chow
a240e150e837b5a95ed19765a2e8b7c5b6013f35 streams: remove AutoFile::Get() entirely (Pieter Wuille) e624a9bef16b6335fd119c10698352b59bf2930a streams: cache file position within AutoFile (Pieter Wuille) Pull request description: Fixes #30833. Instead of relying on frequent `ftell` calls (which appear to cause a significant slowdown on some systems) in XOR-enabled `AutoFile`s, cache the file position within `AutoFile` itself. ACKs for top commit: achow101: ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35 davidgumberg: untested reACK https://github.com/bitcoin/bitcoin/pull/30884/commits/a240e150e837b5a95ed19765a2e8b7c5b6013f35 theStack: Code-review ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35 Tree-SHA512: fd3681edc018afaf955dc7a41a0c953ca80d46c1129e3c5b306c87c95aae93b2fe7b900794eb8b6f10491f9211645e7939918a28838295e6873eb226fca7006f
2024-09-16Merge bitcoin/bitcoin#29436: net: call `Select` with reachable networks in ↵Ava Chow
`ThreadOpenConnections` e4e3b44e9cc7227b3ad765397c884999f57bac2e net: call `Select` with reachable networks in `ThreadOpenConnections` (brunoerg) 829becd990b504a2e8a57fa8a6ff6ac6ae8ff900 addrman: change `Select` to support multiple networks (brunoerg) f698636ec86c004ab331994559c163b7319e6423 net: add `All()` in `ReachableNets` (brunoerg) Pull request description: This PR changes addrman's `Select` to support multiple networks and change `ThreadOpenConnections` to call it with reachable networks. It can avoid unnecessary `Select` calls and avoid exceeding the max number of tries (100), especially when turning a clearnet + Tor/I2P/CJDNS node to Tor/I2P/CJDNS. Compared to #29330, this approach is "less aggresive". It does not add a new init flag and does not impact address relay. I did an experiment of calling `Select` without passing a network until it finds an address from a network that compose 20% ~ 25% of the addrman (limited to 100 tries). ![Screenshot 2024-02-14 at 14 37 58](https://github.com/bitcoin/bitcoin/assets/19480819/7b6863a5-d7a6-40b6-87d5-01667c2de66a) ACKs for top commit: achow101: ACK e4e3b44e9cc7227b3ad765397c884999f57bac2e vasild: ACK e4e3b44e9cc7227b3ad765397c884999f57bac2e naumenkogs: ACK e4e3b44e9cc7227b3ad765397c884999f57bac2e Tree-SHA512: e8466b72b85bbc2ad8bfb14471eb27d2c50d4e84218f5ede2c15a6fa3653af61b488cde492dbd398f7502bd847e95bfee1abb7e01092daba2236d3ce3d6d2268
2024-09-16Merge bitcoin/bitcoin#30410: rpc, rest: Improve block rpc error handling, ↵Ava Chow
check header before attempting to read block data. 6a1aa510e31e8b77793341473aa5afc9d023a6e3 rpc: check block index before reading block / undo data (Martin Zumsande) 6cbf2e5f8197e51b8f3d789ba9f5874a2fd7b93a rpc: Improve gettxoutproof error when only header is available. (Martin Zumsande) 69fc867ea19ba3bd8c38a18b5e3f0e366c46af5b test: add coverage to getblock and getblockstats (Martin Zumsande) 5290cbd58504dcbab97cb0944b3ae0a0f79c4502 rpc: Improve getblock / getblockstats error when only header is available. (Martin Zumsande) e5b537bbdfbfb43ac7bd8e91958e4a2bbd2577ff rest: improve error when only header of a block is available. (Martin Zumsande) Pull request description: Fixes #20978 If a block was pruned, `getblock` already returns a specific error: "Block not available (pruned data)". But if we haven't received the full block yet (e.g. in a race with block downloading after a new block was received headers-first, or during IBD) we just return an unspecific "Block not found on disk" error and log `ERROR: ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0) ` which suggest something went wrong even though this is a completely normal and expected situation. This PR improves the error message and stops calling `ReadRawBlockFromDisk()`, when we already know from the header that the block is not available on disk. Similarly, it prevents all other rpcs from calling blockstorage read functions unless we expect the data to be there, so that `LogError()` will only be thrown when there is an actual file system problem. I'm not completely sure if the cause is important enough to change the wording of the rpc error, that some scripts may rely on. If reviewers prefer it, an alternative solution would be to keep returning the current "Block not found on disk" error, but return it immediately instead of calling `ReadRawBlockFromDisk`, which would at least prevent the log error and also be an improvement in my opinion. ACKs for top commit: fjahr: re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3 achow101: ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3 andrewtoth: re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3 Tree-SHA512: 491aef880e8298a05841c4bf8eb913ef84820d1ad5415fd17d9b441bff181959ebfdd432b5eb8347dc9c568433f9a2384ca9d84cd72c79d8a58323ca117538fe
2024-09-16Merge bitcoin/bitcoin#28358: Drop -dbcache limitAva Chow
bb3b980dfd96d9a89e6b04aef2859ce0555c6807 validation: drop maximum -dbcache (Sjors Provoost) Pull request description: Due to recent UTXO set growth, the current maximum value for `-dbcache` of 16GB is ~just months away from being~ insufficient (for those who wish to complete IBD with the UTXO set held in RAM). This drops the limit. It also adds a warning that it's up to users to check that they have enough RAM. Fixes #28249. --- A previous version of this PR increased the maximum to 64GB. It also made startup abort if the value provided is too high, rather than quietly round it down. But this didn't get much support. ACKs for top commit: achow101: ACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807 tdb3: ACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807 BenWestgate: crACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807. Tree-SHA512: 8515fff468c2387a0b04bd9523ab1df46d6325738588b7550fabddbb8624817a583d95b95ea246407f9f0ff3e43e760cf7334621bec6af79710176328528a3ef
2024-09-16Merge bitcoin/bitcoin#30868: refactor: add clang-tidy ↵Ava Chow
`modernize-use-starts-ends-with` check fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3 tidy: add clang-tidy `modernize-use-starts-ends-with` check (Roman Zeyde) Pull request description: ACKs for top commit: jonatack: re-ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3 only change since my previous ACK is the commit message achow101: ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3 stickies-v: ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3 hebasto: ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3, I have reviewed the code and it looks OK. Tree-SHA512: 334e0ff91b9b108a57cdfc12ee53685b792d377e11124c7c394b8f681a8168a8d65a56c7f884555238e65e97e9ad62ede52b79219ce258979e54abdd76721df1
2024-09-16Merge bitcoin/bitcoin#30440: Have createNewBlock() return a BlockTemplate ↵Ava Chow
interface a93c171faa7b4426823466e972c8f24260918bbf Drop unneeded nullptr check from CreateNewBlock() (Sjors Provoost) dd87b6dff352c408e90e3d612d773ff9562d2382 Have createNewBlock return BlockTemplate interface (Sjors Provoost) Pull request description: Suggested in https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2225337100 An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data. This would be the case for a Stratum v2 Template Provider which needs to send a [NewTemplate](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#72-newtemplate-server---client) message message (which doesn't include transactions) as quickly as possible. It does not include the serialized block transactions. ACKs for top commit: achow101: ACK a93c171faa7b4426823466e972c8f24260918bbf ryanofsky: Code review ACK a93c171faa7b4426823466e972c8f24260918bbf. Since last review, just rebased with no changes or conflicts itornaza: Code review ACK a93c171faa7b4426823466e972c8f24260918bbf TheCharlatan: Re-ACK a93c171faa7b4426823466e972c8f24260918bbf Tree-SHA512: 17cb61eb5548e9d4a69e34dd732f68a06cde2ad3d82c8339efee704c7860d5de144d93b23d6ecd6ee4ec205844e5560ad0f6d3917822fa75bb8e640c5f51af9a
2024-09-16Merge bitcoin/bitcoin#30661: fuzz: Test headers pre-sync through p2pglozow
a97f43d63a6e835bae20b0bc5d536df98f55d8a0 fuzz: Add harness for p2p headers sync (marcofleon) a0eaa4749fe0f755e113eee70dee1989bdc07ad5 Add FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION in PoW check (marcofleon) a3f6f5acd89f2f5bb136ec247f259d212e8944d0 build: Automatically define FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for fuzz builds (marcofleon) 0c02d4b2bdbc7a3fc3031a63b3b16bafa669d51c net_processing: Make MAX_HEADERS_RESULTS a PeerManager option (marcofleon) Pull request description: This PR reopens https://github.com/bitcoin/bitcoin/pull/28043. It's a regression fuzz test for https://github.com/bitcoin/bitcoin/pull/26355 and [a couple bugs](https://github.com/bitcoin/bitcoin/pull/25717/commits/ed6cddd98e32263fc116a4380af6d66da20da990) that were addressed in https://github.com/bitcoin/bitcoin/pull/25717. This should help us move forward with the [removal of mainnet checkpoints](https://github.com/bitcoin/bitcoin/pull/25725). It seems like the main concern in https://github.com/bitcoin/bitcoin/pull/28043 was the global mock function for proof of work. This PR aims to be an improvement by replacing the previous approach with a fuzz build configured using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. This ensures that the simplified test code will never be in a release binary. If we agree this is the way to go, there are some other places (for future targets) where this method could be used. In this target, PoW isn't being tested, so the goal is to bypass the check and let the fuzzer do its thing. In the other harnesses where PoW is actually being fuzzed, `CheckProofOfWork` is now `CheckProofOfWorkImpl`. So, the only change to that function is in the name. More about `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` can be found at https://llvm.org/docs/LibFuzzer.html#fuzzer-friendly-build-mode and https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#d-modifying-the-target. ACKs for top commit: naumenkogs: ACK a97f43d63a6e835bae20b0bc5d536df98f55d8a0 dergoegge: reACK a97f43d63a6e835bae20b0bc5d536df98f55d8a0 instagibbs: tested ACK a97f43d63a6e835bae20b0bc5d536df98f55d8a0 brunoerg: ACK a97f43d63a6e835bae20b0bc5d536df98f55d8a0 Tree-SHA512: 60b0bc6aadd8ca4c39db9cbba2da2debaaf68afcb6a8dd75c1ce48ca9e3996948fda8020930b6771a424e0f7c41b0b1068db4aa7dbe517f8fc152f1f712058ad
2024-09-16Merge bitcoin/bitcoin#30286: cluster mempool: optimized candidate searchglozow
9ad2fe7e69e9e69949ebbb280a15756dc3301f09 clusterlin: only start/use search when enough iterations left (Pieter Wuille) bd044356edb6c8978df600f33b0073f772ee047c clusterlin: improve heuristic to decide split transaction (optimization) (Pieter Wuille) 71f26293988019d2035bcc55af7b440b494b56bc clusterlin: include topological pot subsets automatically (optimization) (Pieter Wuille) e20fda77a2da1da3deb273d83700a0b7139422ab clusterlin: reduce computation of unnecessary pot sets (optimization) (Pieter Wuille) 6060a948caf6dbc7505658d6cac750e25698eaf9 clusterlin bench: add example hard cluster benchmarks (Pieter Wuille) 2965fbf203f0b244814d7159381a2e9472bc1f97 clusterlin: track upper bound potential set for work items (optimization) (Pieter Wuille) 9e43e4ce109e98a1ea3f54bbb4de86bc1b92ae4f clusterlin: use feerate-sorted depgraph in SearchCandidateFinder (Pieter Wuille) b80e6dfe780b3678bb41f2d9d63816f097529b2e clusterlin: add reordering support for DepGraph (Pieter Wuille) 85a285a306100d1815c4ad0f4e52ccbcae8b0fbc clusterlin: separate initial search entries per component (optimization) (Pieter Wuille) e4faea9ca79ce1f277d8499962c7de64f26b0436 clusterlin bench: have low/high iter benchmarks instead of per-iter (Pieter Wuille) Pull request description: Part of cluster mempool: #30289 Depends on #30126, and was split off from it. This improves the candidate search algorithm introduced in the previous PR with a variety of optimizations. The resulting search algorithm largely follows Section 2 of [How to linearize your cluster](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-2-finding-high-feerate-subsets-5), though with a few changes: * Connected component analysis is performed inside the search algorithm (creating initial work items per component for each candidate), rather than once at a higher level. This duplicates some work but is significantly simpler in implementation. * No ancestor-set based presplitting inside the search is performed; instead, the `best` value is initialized with the best topologically valid set known to the LIMO algorithm before search starts: the better one out of the highest-feerate remaining ancestor set, and the highest-feerate prefix of remaining transactions in `old_linearization`. * Work items are represented using an included set *inc* and an undefined set *und*, rather than included and excluded. * Potential sets *pot* are not computed for work items with empty *inc*. At a high level, the only missing optimization from that post is bottleneck analysis; my thinking is that it only really helps with clusters that are already relatively cheap to linearize (doing so would need to be done at a higher level, not inside the search algorithm). --- Overview of the impact of each commit here on linearize performance: * **[clusterlin bench: have low/high iter benchmarks instead of per-iter](https://github.com/bitcoin/bitcoin/pull/30286/commits/21a184db63bd13cf0c2dff9e6e61ca85f2ff4454)**: no impact * **[separate initial search entries per component (optimization)](https://github.com/bitcoin/bitcoin/pull/30286/commits/c84c5c86ba6c12ce11e997d0c59d60694c667488)**: reduce iterations, increase start-up cost * **[add reordering support for DepGraph](https://github.com/bitcoin/bitcoin/pull/30286/commits/019ff2960976b677d91bab47e75be6ed180b37a3)**: no impact * **[use feerate-sorted depgraph in SearchCandidateFinder](https://github.com/bitcoin/bitcoin/pull/30286/commits/8e27dd5a220237da5d362376db6406e47f063776)**: typically reduce iterations, increase start-up cost * **[track upper bound potential set for work items](https://github.com/bitcoin/bitcoin/pull/30286/commits/781e0fb3aa5cbd0983b01547481eefa0804dfb5b)**: reduce iterations, increase cost per iteration * **[reduce computation of unnecessary pot sets](https://github.com/bitcoin/bitcoin/pull/30286/commits/9fe834fa979c8ab038aec57dac2f468ca70c41a9)**: reduce cost per iteration * **[include topological pot subsets automatically](https://github.com/bitcoin/bitcoin/pull/30286/commits/30612710a4e5fc8c950bd1ff4ab9fa843b59132d)**: reduce iterations, increase cost per iteration * **[improve heuristic to decide split transaction](https://github.com/bitcoin/bitcoin/pull/30286/commits/1880c00ab1661dbfc867f69e7556b28bc3cf7a42)**: typically reduce iterations, increase cost per iteration * **[only start/use search when enough iterations left](https://github.com/bitcoin/bitcoin/pull/30286/commits/12760a57b3a6cd1aeb3b7532311f648de2b45aa4)**: just account for start-up cost as equivalent iterations ACKs for top commit: sdaftuar: ACK 9ad2fe7e69e9e69949ebbb280a15756dc3301f09 instagibbs: reACK 9ad2fe7e69e9e69949ebbb280a15756dc3301f09 glozow: reACK 9ad2fe7e69e, just have a question about the docs Tree-SHA512: 108bcbb0676f36071eb83954059b5f3d6646c745015b644a2a5d7f5a8ac9424c2d01d339fa6318a3aff4cf313308e85bb80b0090899720a3fcba027b8025590a
2024-09-16Merge bitcoin/bitcoin#30899: qt: Translations updatemerge-script
ae0529576147a1a5bee992574e2cefc8a1fa37d0 qt: Translations update (Hennadii Stepanov) Pull request description: The recent translations from Transifex.com fetched with the bitcoin-maintainer-tools/update-translations.py tool. Fixes https://github.com/bitcoin/bitcoin/issues/30897. Translations are updated, while skipping removing translations for 2 languages. Related: - https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349332544 ACKs for top commit: achow101: ACK ae0529576147a1a5bee992574e2cefc8a1fa37d0 pablomartin4btc: ACK ae0529576147a1a5bee992574e2cefc8a1fa37d0 Tree-SHA512: 52107dfccaaaef187389ecb114eef4283ff40a3b855de811685fc2d6cfb1d869b2610edf86b25a235266fd7dcb36f693a6816a60639246b5d439c4f6482b2ebd
2024-09-14tidy: add clang-tidy `modernize-use-starts-ends-with` checkRoman Zeyde
2024-09-13streams: remove AutoFile::Get() entirelyPieter Wuille
Co-Authored-By: David Gumberg <davidzgumberg@gmail.com>
2024-09-13Merge bitcoin/bitcoin#30896: kernel: Move background load thread to node contextAva Chow
bc7900f33db3d01fb93dfee7981c01ea495cd42e kernel: Move background load thread to node context (TheCharlatan) Pull request description: The thread handle is never used by the ChainstateManager, so move it out and into the node context. Users of the kernel library now no longer have to manually join the thread when destructing the ChainstateManager. ACKs for top commit: maflcko: ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e 🔄 achow101: ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e ryanofsky: Code review ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e. Nice cleanup jonatack: Light ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e stickies-v: ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e Tree-SHA512: add9c4823731324e3db50f95e023e99d55db7cc75c69083ae7c9c2157e5540968caa6cf10674aa4901f91366b02ebb1ff18bb977fec0a46431e2196448958b9d
2024-09-13Merge bitcoin/bitcoin#30892: test: Check already deactivated network stays ↵Ava Chow
suspended after dumptxoutset 72c9a1fe94f927220d3159f516fda684ae9d4caa test: Check that network stays suspended after dumptxoutset if it was off before (Fabian Jahr) Pull request description: Follow-up to #30817 which covered the robustness of `dumptxoutset`: network is deactivated during the run but re-activated even when an issue was encountered. But it did not cover the case if the user had deactivated the network themselves before. In that case the user may want the network to stay off so the network is not reactivated after `dumptxoutset` finishes. A test for this behavior is added here. ACKs for top commit: achow101: ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa pablomartin4btc: ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa theStack: utACK 72c9a1fe94f927220d3159f516fda684ae9d4caa tdb3: tested ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa Tree-SHA512: 18a57c5782e99a018414db0597e88debeb32666712c2a75dddbb55135d8f1ddd1eeacba8bbbd35fc03b6c4ab0522fe074ec08edea729560b018f51efabf00e89
2024-09-13Merge bitcoin/bitcoin#30233: refactor: move m_is_inbound out of CNodeStateAva Chow
07f4cebe5780f1039541d989e64b70eccc5b4eb5 refactor: move m_is_inbound out of CNodeState (Sergi Delgado Segura) Pull request description: `m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we are currently storing it in `CNodeState`, which requires locking `cs_main` in order to access it. This can be moved to the outside scope and only require `m_peer_mutex`. This is a refactor in preparation for Erlay reworks. ACKs for top commit: maflcko: ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5 🗞 achow101: ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5 marcofleon: ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5 naumenkogs: ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5 Tree-SHA512: bcc77135646c697204a4605971774cb3ccdf11b3e363a7339bfb4d1678de1681d6ca642dc467f7d71834a94dd56e05367e7fd32a3e6dc6be30c89342f39bf695
2024-09-13qt: Translations updateHennadii Stepanov
The recent translations from Transifex.com 28.x fetched with the bitcoin-maintainer-tools/update-translations.py tool.
2024-09-13Merge bitcoin/bitcoin#30877: code style: update .editorconfig filemerge-script
95560616fbab3ddca9d85980b7f73c8a816bc99e code style: update .editorconfig file (Sebastian Falbesoner) Pull request description: Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d57) w.r.t. following changes: - consider Rust .rs files (relevant since #28076, commit bbbbdb0c) - reflect build system change to CMake (#30454, #30664) - add setting for bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim). The PR is made under the assumption that the file is still considered useful, especially for new contributors. If people feel like that's not the case anymore, the alternative is to delete it, obviously. Top commit has no ACKs. Tree-SHA512: 8406b1caf31e310f7e17c607d97beac583481e71b4425e0be2bbd8207096aa374a70151b58aae5fdb648ef5ff5c7e1d0a2949e6de3355bdd2009d8353ee24af0
2024-09-13Merge bitcoin/bitcoin#30890: doc: unit test runner help fixup and improvementsmerge-script
282f0e92559da23e356504a564a0322b9888e50b Unit test runner documentation fix and improvements (Jon Atack) Pull request description: Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that. Improve the content and order of the unit test documentation. ACKs for top commit: pablomartin4btc: re-ACK 282f0e92559da23e356504a564a0322b9888e50b tdb3: re ACK 282f0e92559da23e356504a564a0322b9888e50b Tree-SHA512: 0d25108ab641bcd9b53f99d139afeec90a34f44d5b00c3c677f7539d87782440a28fadc348663b8c28ace43552834737b9c1e8f5517c68edc8547695a9cb5063
2024-09-13code style: update .editorconfig fileSebastian Falbesoner
Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d57) w.r.t. following changes: - consider Rust .rs files (relevant since #28076, commit bbbbdb0c) - reflect build system change to CMake (#30454, #30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim).
2024-09-13Unit test runner documentation fix and improvementsJon Atack
- Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that. - Improve the content and order of the unit test documentation.
2024-09-13rpc: check block index before reading block / undo dataMartin Zumsande
This avoids low-level log errors that are supposed to only occur when there is an actual problem with the block on disk missing unexpectedly, but not in the case where the block and/or undo data are expected not to be there. It changes behavior such that in the first case (block index indicates data is available but retrieving it fails) an error is thrown. It also adjusts a functional tests that tried to simulate not having undo data (but having block data) by deleting the undo file. This situation should occur reality because block and undo data are pruned together. Instead, test this situation with a block that hasn't been connected.
2024-09-13rpc: Improve gettxoutproof error when only header is available.Martin Zumsande
2024-09-13test: add coverage to getblock and getblockstatsMartin Zumsande
also removes an unnecessary newline. Co-authored-by: tdb3 <106488469+tdb3@users.noreply.github.com>
2024-09-13rpc: Improve getblock / getblockstats error when only header is available.Martin Zumsande
This improves the error message of the getblock and getblockstats rpc and prevents calls to ReadRawBlockFromDisk(), which are unnecessary if we know from the header nStatus field that the block is not available.
2024-09-13rest: improve error when only header of a block is available.Martin Zumsande
This avoids calling ReadRawBlockFromDisk() when the block is expected not to be available because we haven't downloaded it yet and only know the header.
2024-09-13Merge bitcoin/bitcoin#30433: build: add `standard branch-protection` to ↵merge-script
hardening flags for aarch64-linux 001b1cf010453adbb1316a6fa8911398953afe61 build: use standard branch-protection for aarch64-linux (fanquake) Pull request description: Use `-mbranch-protection=standard` when targetting `*aarch64-*-linux*`. Part of #24123, but this flag can already be used on a best effort basis. Note that this flag is also already used by default, in the toolchain, on various distros (i.e Fedora). ACKs for top commit: hebasto: ACK 001b1cf010453adbb1316a6fa8911398953afe61. TheCharlatan: ACK 001b1cf010453adbb1316a6fa8911398953afe61 Tree-SHA512: 2d7ae60f59921a62d51139cb0fd5cecbed4f63266564b2623b7d160f5b0c2c42c78ef8aeff787f485eccc46a9ffd5da70023ec093df6add7c982e0d48a1601b5
2024-09-13kernel: Move background load thread to node contextTheCharlatan
The thread handle is never used by the ChainstateManager, so move it out and into the node context. Users of the kernel library now no longer have to manually join the thread when destructing the ChainstateManager.
2024-09-13streams: cache file position within AutoFilePieter Wuille
2024-09-13Merge bitcoin-core/gui#835: Fix crash when closing walletHennadii Stepanov
a965f2bc07a3588f8c2b8d6a542961562e3f5d0e gui: fix crash when closing wallet (furszy) Pull request description: The crash occurs because `WalletController::removeAndDeleteWallet` is called twice for the same wallet model: first in the GUI's button connected function `WalletController::closeWallet`, and then again when the backend emits the `WalletModel::unload` signal. This causes the issue because `removeAndDeleteWallet` inlines an `erase(std::remove())`. So, if `std::remove` returns an iterator to the end (indicating the element wasn't found because it was already erased), the subsequent call to `erase` leads to an undefined behavior. Test Notes: Try closing any wallet using the toolbar button in the GUI. It will crash in master, but not here. Fixes https://github.com/bitcoin/bitcoin/issues/30887. ACKs for top commit: pablomartin4btc: tACK a965f2bc07a3588f8c2b8d6a542961562e3f5d0e jarolrod: ACK a965f2bc07a3588f8c2b8d6a542961562e3f5d0e hebasto: ACK a965f2bc07a3588f8c2b8d6a542961562e3f5d0e. Tree-SHA512: c94681b95cb566f7aabd0d4fb10f797c2cea6ac569abc265e918f08e6abae3335432a0b0879372b54b2c109798ed0a4a249bf162c34add59cbd18d38a2d9660e
2024-09-13build: use standard branch-protection for aarch64-linuxfanquake
2024-09-13guix: build Linux GCC with --enable-cetfanquake
Similar to #29695, and in the same vein of explicitly configuring hardening options in our release toolchain. See https://gcc.gnu.org/install/configure.html: > Enable building target run-time libraries with control-flow instrumentation, > see `-fcf-protection option`. When --enable-cet is specified target > libraries are configured to add `-fcf-protection` and, if needed, > other target specific options to a set of building options. > `--enable-cet=auto` is default. CET is enabled on Linux/x86 if target > binutils supports Intel CET instructions and disabled otherwise. > In this case, the target libraries are configured to get additional > `-fcf-protection` option.
2024-09-13Drop unneeded nullptr check from CreateNewBlock()Sjors Provoost
2024-09-13Have createNewBlock return BlockTemplate interfaceSjors Provoost
An external program that uses the Mining interface may need quick access to some information in the block template, while it can wait a bit longer for the full raw transaction data. This would be the case for a Stratum v2 Template Provider which needs to send a NewTemplate message (which doesn't include transactions) as quickly as possible.
2024-09-12gui: fix crash when closing walletfurszy
The crash occurs because 'WalletController::removeAndDeleteWallet' is called twice for the same wallet model: first in the GUI's button connected function 'WalletController::closeWallet', and then again when the backend emits the 'WalletModel::unload' signal. This causes the issue because 'removeAndDeleteWallet' inlines an erase(std::remove()). So, if 'std::remove' returns an iterator to the end (indicating the element wasn't found because it was already erased), the subsequent call to 'erase' leads to an undefined behavior.
2024-09-12test: Check that network stays suspended after dumptxoutset if it was off beforeFabian Jahr
2024-09-12clusterlin: only start/use search when enough iterations leftPieter Wuille
2024-09-12clusterlin: improve heuristic to decide split transaction (optimization)Pieter Wuille
Empirically, this approach seems to be more efficient in common real-life clusters, and does not change the worst case. Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
2024-09-12clusterlin: include topological pot subsets automatically (optimization)Pieter Wuille
Automatically add topologically-valid subsets of the potential set pot to inc. It can be proven that these must be part of the best reachable topologically-valid set from that work item. This is a crucial optimization that (apparently) reduces the maximum number of iterations from ~2^(N-1) to ~sqrt(2^N). Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
2024-09-12clusterlin: reduce computation of unnecessary pot sets (optimization)Pieter Wuille
Keep track of which transactions in the graph have an individual feerate that is better than the best included set so far. Others do not need to be added to the pot set, as they cannot possibly help beating best.
2024-09-12clusterlin bench: add example hard cluster benchmarksPieter Wuille
Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
2024-09-12clusterlin: track upper bound potential set for work items (optimization)Pieter Wuille
In each work item, keep track of a conservative overestimate of the best possible feerate that can be reached from it, and then use these to avoid exploring hopeless work items.
2024-09-12clusterlin: use feerate-sorted depgraph in SearchCandidateFinderPieter Wuille
This is a requirement for a future commit, which will rely on quickly iterating over transaction sets in decreasing individual feerate order.
2024-09-12clusterlin: add reordering support for DepGraphPieter Wuille
Add a DepGraph(depgraph, reordering) function that constructs a new DepGraph corresponding to an old one, but with its transactions is a modified order (given as a vector from old to new positions). Also use this reordering feature inside DepGraphFormatter::Unser, which needs a small modification so that its reordering mapping is old-to-new (rather than the new-to-old it used before).
2024-09-12clusterlin: separate initial search entries per component (optimization)Pieter Wuille
Before this commit, the worst case for linearization involves clusters which break apart in several smaller components after the first candidate is included in the output linearization. Address this by never considering work items that span multiple components of what remains of the cluster.
2024-09-12clusterlin bench: have low/high iter benchmarks instead of per-iterPieter Wuille
2024-09-12Merge bitcoin/bitcoin#30880: test: Wait for local services to update in ↵Ava Chow
feature_assumeutxo 19f4a7c95a99162122068d4badffeea240967a65 test: Wait for local services to update in feature_assumeutxo (Fabian Jahr) Pull request description: Closes #30878 It seems like there is a race where the block is stored locally and `getblock` does not error anymore, but `ActivateBestChain` has not finished yet, so the local services are not updated yet either. Fix this by waiting for the local services to update. Can be reproduced locally by adding the sleep here: ```cpp ──────────────────────────────────────────────────────────────────────────────────────────────────────────┐ src/validation.cpp:3567: bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< │ ──────────────────────────────────────────────────────────────────────────────────────────────────────────┘ } if (WITH_LOCK(::cs_main, return m_disabled)) { std::this_thread::sleep_for(std::chrono::seconds(10)); // Background chainstate has reached the snapshot base block, so exit. // Restart indexes to resume indexing for all blocks unique to the snapshot ``` ACKs for top commit: maflcko: review-only ACK 19f4a7c95a99162122068d4badffeea240967a65 achow101: ACK 19f4a7c95a99162122068d4badffeea240967a65 pablomartin4btc: tACK 19f4a7c95a99162122068d4badffeea240967a65 furszy: Code review ACK [19f4a7c](https://github.com/bitcoin/bitcoin/pull/30880/commits/19f4a7c95a99162122068d4badffeea240967a65). Tree-SHA512: 70dad3795988956c5e20f2d2d895fb56c5e3ce257c7547d3fd729c88949f0e24cb34594da1537bce8794ad02b2db44e7e46e995aa32539cd4dd84c4f1d4bb1c4
2024-09-12Merge bitcoin/bitcoin#30546: util: Use consteval checked format string in ↵Ryan Ofsky
FatalErrorf, LogConnectFailure fa5bc450d5d4c1d2daf7b205f1678402c3c21678 util: Use compile-time check for LogConnectFailure (MarcoFalke) fa7087b896c0150c29d7a27c53e0533831a2bf3b util: Use compile-time check for FatalErrorf (MarcoFalke) faa62c0112f2b7ab69c80a5178f5b79217bed0a6 util: Add ConstevalFormatString (MarcoFalke) fae7b83eb58d22ed83878561603991131372cdd7 lint: Remove forbidden functions from lint-format-strings.py (MarcoFalke) Pull request description: The `test/lint/lint-format-strings.py` was designed to count the number of format specifiers and assert that they are equal to the number of parameters passed to the format function. The goal seems reasonable, but the implementation has many problems: * It is written in Python, meaning that C++ code can not be parsed correctly. Currently it relies on brittle regex and string parsing. * Apart from the parsing errors, there are also many logic errors. For example, `count_format_specifiers` allows a mix of positional specifiers and non-positional specifiers, which can lead to runtime format bugs. Also, `count_format_specifiers` silently skipped over "special" format specifiers, which are valid in tinyformat, which again can lead to runtime format bugs being undetected. * The brittle logic has a history of breaking in pull requests that are otherwise fine. This causes the CI to fail and the pull request being blocked from progress until the bug in the linter is fixed, or the code is rewritten to work around the bug. * It is only run in the CI, or when the developer invokes the script. It would be better if the developer got the error message at compile-time, directly when writing the code. Fix all issues by using a `consteval` checked format string in `FatalErrorf` and `LogConnectFailure`. This is the first step toward https://github.com/bitcoin/bitcoin/issues/30530 and a follow-up will apply the approach to the other places. ACKs for top commit: stickies-v: re-ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 l0rinc: ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 hodlinator: ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 ryanofsky: Code review ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 Tree-SHA512: d6189096b16083143687ed1b1559cf4f92f97dd87bc5d00673e44f4fb9fce7bb7b215cfdfc39b6e6a24f0b75a79a03ededce966639e554f7172e1fc22cf015ae
2024-09-12Merge bitcoin/bitcoin#30618: test: support std::optional in BOOST_CHECK_* ↵Ryan Ofsky
and increase FromUserHex fuzz feature coverage 1eac96a503b6bac3eaf5d0eb3d23ffde3bfbf9aa Compare FromUserHex result against other hex validators and parsers (Lőrinc) 19947863e16425e6a119e7a4819867292b1235ee Use BOOST_CHECK_EQUAL for optional, arith_uint256, uint256, uint160 (Lőrinc) 743ac30e349e181c26a2d2af0bcb93b0835ce521 Add std::optional support to Boost's equality check (Lőrinc) Pull request description: Enhanced `FromUserHex` coverage by: * Added `std::optional` support to `BOOST_CHECK_EQUAL`, allowing direct comparisons of `std::optional<T>` with other `T` expected values. * Increased fuzz testing for hex parsing to validate against other hex validators and parsers. ---- * Use BOOST_CHECK_EQUAL for https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706637780 arith_uint256, uint256, uint160 Example error before: > unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_optional_access test/validation_chainstatemanager_tests.cpp:781: last checkpoint after: > test/validation_chainstatemanager_tests.cpp:801: error: in "validation_chainstatemanager_tests/chainstatemanager_args": check set_opts({"-assumevalid=0"}).assumed_valid_block == uint256::ZERO has failed [std::nullopt != 0000000000000000000000000000000000000000000000000000000000000000] ACKs for top commit: stickies-v: re-ACK 1eac96a503b6bac3eaf5d0eb3d23ffde3bfbf9aa ryanofsky: Code review ACK 1eac96a503b6bac3eaf5d0eb3d23ffde3bfbf9aa. Only changes since last review were auto type and fuzz test tweaks. hodlinator: ACK 1eac96a503b6bac3eaf5d0eb3d23ffde3bfbf9aa Tree-SHA512: f1d2c65f0ee4e97830700be5b330189207b11ed0c89a8cebf0f97d43308402a6b3732e10130c79a0c044f7d2eeabfb5359990825aadf02c4ec19428dcd982b00
2024-09-12Merge bitcoin/bitcoin#30883: build: Revert "Minimize I/O operations in ↵merge-script
`GenerateHeaderFrom{Json,Raw}.cmake`" fdeb717e78fd55fbfda199c87002358bdc5895af Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`" (Hennadii Stepanov) Pull request description: This reverts commit b07fe666f27e2b2515d2cb5a0339512045e64761 from https://github.com/bitcoin/bitcoin/pull/30842. Fixes https://github.com/bitcoin/bitcoin/issues/30881. Apparently, the `string(APPEND ...)` command isn't optimized for large strings. ACKs for top commit: maflcko: review ACK fdeb717e78fd55fbfda199c87002358bdc5895af Tree-SHA512: ad5c3d49d3395ab318edcd7c9a98090838bec0cd3c1f1cc6ebc6f4262df2494f605458b523251bf5e590bbcfda15ed963f0a814678135ce4cc2dca9a108d20c7
2024-09-12Merge bitcoin/bitcoin#30814: kernel: Create usable static kernel librarymerge-script
0dd16d7118f10ac291a45644769121cbdfd2352f build: Add a pkg-config file for libbitcoinkernel (TheCharlatan) 45be32f8384398ad8d590137d05a6373aa827b75 build: Produce a usable static kernel library (TheCharlatan) Pull request description: Since the move to cmake, the kernel static library that is installed after a cmake --install build is unusable. It lacks symbols for the internal libraries, besides those defined in the kernel library target. Fix this by explicitly installing all the required internal static libraries. To make usage of these installed libraries easy, add a pkg-config file that can be used during linking. This patch can be tested with: ``` cmake -B build -DBUILD_SHARED_LIBS=OFF -DBUILD_KERNEL_LIB=ON cmake --build build cmake --install build g++ -std=c++20 -o test_chainstate src/bitcoin-chainstate.cpp -I/home/drgrid/bitcoin/src $(pkg-config --libs --static libbitcoinkernel) ``` Attempts to solve #30801 ACKs for top commit: hebasto: ACK 0dd16d7118f10ac291a45644769121cbdfd2352f. fanquake: ACK 0dd16d7118f10ac291a45644769121cbdfd2352f - this looks like a good place to start. ryanofsky: Code review ACK 0dd16d7118f10ac291a45644769121cbdfd2352f Tree-SHA512: 92f7bc959584bdc595f4aa6d0ab133355481075fe8564224fd7ac122fd7bdd75f98cf26ef0a6a7d84fd552d2258ddca1b674eca91122469a58bacc5f0a0ec2ef
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-30 03:54:27 +0000