Crazecdwn #1
Open
Crazecdwn #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
also removes an unnecessary newline. Co-authored-by: tdb3 <[email protected]>
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.
- 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.
Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since #28076, commit bbbbdb0) - 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).
282f0e9 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 282f0e9 tdb3: re ACK 282f0e9 Tree-SHA512: 0d25108ab641bcd9b53f99d139afeec90a34f44d5b00c3c677f7539d87782440a28fadc348663b8c28ace43552834737b9c1e8f5517c68edc8547695a9cb5063
9556061 code style: update .editorconfig file (Sebastian Falbesoner) Pull request description: Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d5) w.r.t. following changes: - consider Rust .rs files (relevant since #28076, commit bbbbdb0) - 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
The recent translations from Transifex.com 28.x fetched with the bitcoin-maintainer-tools/update-translations.py tool.
07f4ceb 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 07f4ceb 🗞 achow101: ACK 07f4ceb marcofleon: ACK 07f4ceb naumenkogs: ACK 07f4ceb Tree-SHA512: bcc77135646c697204a4605971774cb3ccdf11b3e363a7339bfb4d1678de1681d6ca642dc467f7d71834a94dd56e05367e7fd32a3e6dc6be30c89342f39bf695
… after dumptxoutset 72c9a1f 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 72c9a1f pablomartin4btc: ACK 72c9a1f theStack: utACK 72c9a1f tdb3: tested ACK 72c9a1f Tree-SHA512: 18a57c5782e99a018414db0597e88debeb32666712c2a75dddbb55135d8f1ddd1eeacba8bbbd35fc03b6c4ab0522fe074ec08edea729560b018f51efabf00e89
bc7900f 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 bc7900f 🔄 achow101: ACK bc7900f ryanofsky: Code review ACK bc7900f. Nice cleanup jonatack: Light ACK bc7900f stickies-v: ACK bc7900f Tree-SHA512: add9c4823731324e3db50f95e023e99d55db7cc75c69083ae7c9c2157e5540968caa6cf10674aa4901f91366b02ebb1ff18bb977fec0a46431e2196448958b9d
Co-Authored-By: David Gumberg <[email protected]>
ae05295 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 #30897. Translations are updated, while skipping removing translations for 2 languages. Related: - #30827 (comment) ACKs for top commit: achow101: ACK ae05295 pablomartin4btc: ACK ae05295 Tree-SHA512: 52107dfccaaaef187389ecb114eef4283ff40a3b855de811685fc2d6cfb1d869b2610edf86b25a235266fd7dcb36f693a6816a60639246b5d439c4f6482b2ebd
This may be useful. For example, to store the directory in a specific place, instead of having to use a volume. Possibly, but not limited to sharing a cache: https://ccache.dev/manual/4.10.1.html#_sharing_a_local_cache
9ad2fe7 clusterlin: only start/use search when enough iterations left (Pieter Wuille) bd04435 clusterlin: improve heuristic to decide split transaction (optimization) (Pieter Wuille) 71f2629 clusterlin: include topological pot subsets automatically (optimization) (Pieter Wuille) e20fda7 clusterlin: reduce computation of unnecessary pot sets (optimization) (Pieter Wuille) 6060a94 clusterlin bench: add example hard cluster benchmarks (Pieter Wuille) 2965fbf clusterlin: track upper bound potential set for work items (optimization) (Pieter Wuille) 9e43e4c clusterlin: use feerate-sorted depgraph in SearchCandidateFinder (Pieter Wuille) b80e6df clusterlin: add reordering support for DepGraph (Pieter Wuille) 85a285a clusterlin: separate initial search entries per component (optimization) (Pieter Wuille) e4faea9 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 9ad2fe7 instagibbs: reACK 9ad2fe7 glozow: reACK 9ad2fe7, just have a question about the docs Tree-SHA512: 108bcbb0676f36071eb83954059b5f3d6646c745015b644a2a5d7f5a8ac9424c2d01d339fa6318a3aff4cf313308e85bb80b0090899720a3fcba027b8025590a
a97f43d fuzz: Add harness for p2p headers sync (marcofleon) a0eaa47 Add FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION in PoW check (marcofleon) a3f6f5a build: Automatically define FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for fuzz builds (marcofleon) 0c02d4b net_processing: Make MAX_HEADERS_RESULTS a PeerManager option (marcofleon) Pull request description: This PR reopens #28043. It's a regression fuzz test for #26355 and [a couple bugs](ed6cddd) that were addressed in #25717. This should help us move forward with the [removal of mainnet checkpoints](#25725). It seems like the main concern in #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 a97f43d dergoegge: reACK a97f43d instagibbs: tested ACK a97f43d brunoerg: ACK a97f43d Tree-SHA512: 60b0bc6aadd8ca4c39db9cbba2da2debaaf68afcb6a8dd75c1ce48ca9e3996948fda8020930b6771a424e0f7c41b0b1068db4aa7dbe517f8fc152f1f712058ad
a93c171 Drop unneeded nullptr check from CreateNewBlock() (Sjors Provoost) dd87b6d Have createNewBlock return BlockTemplate interface (Sjors Provoost) Pull request description: Suggested in #29432 (comment) 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 a93c171 ryanofsky: Code review ACK a93c171. Since last review, just rebased with no changes or conflicts itornaza: Code review ACK a93c171 TheCharlatan: Re-ACK a93c171 Tree-SHA512: 17cb61eb5548e9d4a69e34dd732f68a06cde2ad3d82c8339efee704c7860d5de144d93b23d6ecd6ee4ec205844e5560ad0f6d3917822fa75bb8e640c5f51af9a
…h` check fc7b507 tidy: add clang-tidy `modernize-use-starts-ends-with` check (Roman Zeyde) Pull request description: ACKs for top commit: jonatack: re-ACK fc7b507 only change since my previous ACK is the commit message achow101: ACK fc7b507 stickies-v: ACK fc7b507 hebasto: ACK fc7b507, I have reviewed the code and it looks OK. Tree-SHA512: 334e0ff91b9b108a57cdfc12ee53685b792d377e11124c7c394b8f681a8168a8d65a56c7f884555238e65e97e9ad62ede52b79219ce258979e54abdd76721df1
bb3b980 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 bb3b980 tdb3: ACK bb3b980 BenWestgate: crACK bb3b980. Tree-SHA512: 8515fff468c2387a0b04bd9523ab1df46d6325738588b7550fabddbb8624817a583d95b95ea246407f9f0ff3e43e760cf7334621bec6af79710176328528a3ef
…er before attempting to read block data. 6a1aa51 rpc: check block index before reading block / undo data (Martin Zumsande) 6cbf2e5 rpc: Improve gettxoutproof error when only header is available. (Martin Zumsande) 69fc867 test: add coverage to getblock and getblockstats (Martin Zumsande) 5290cbd rpc: Improve getblock / getblockstats error when only header is available. (Martin Zumsande) e5b537b 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 6a1aa51 achow101: ACK 6a1aa51 andrewtoth: re-ACK 6a1aa51 Tree-SHA512: 491aef880e8298a05841c4bf8eb913ef84820d1ad5415fd17d9b441bff181959ebfdd432b5eb8347dc9c568433f9a2384ca9d84cd72c79d8a58323ca117538fe
…enConnections` e4e3b44 net: call `Select` with reachable networks in `ThreadOpenConnections` (brunoerg) 829becd addrman: change `Select` to support multiple networks (brunoerg) f698636 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).  ACKs for top commit: achow101: ACK e4e3b44 vasild: ACK e4e3b44 naumenkogs: ACK e4e3b44 Tree-SHA512: e8466b72b85bbc2ad8bfb14471eb27d2c50d4e84218f5ede2c15a6fa3653af61b488cde492dbd398f7502bd847e95bfee1abb7e01092daba2236d3ce3d6d2268
a240e15 streams: remove AutoFile::Get() entirely (Pieter Wuille) e624a9b 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 a240e15 davidgumberg: untested reACK a240e15 theStack: Code-review ACK a240e15 Tree-SHA512: fd3681edc018afaf955dc7a41a0c953ca80d46c1129e3c5b306c87c95aae93b2fe7b900794eb8b6f10491f9211645e7939918a28838295e6873eb226fca7006f
Also signal m_tip_block_cv when StopRPC is called, for consistency with g_best_block_cv. This is handled in StopRPC instead of OnRPCStopped() because the latter is deleted in a later commit. Co-authored-by: TheCharlatan <[email protected]> Co-authored-by: Ryan Ofsky <[email protected]>
fa22e5c refactor: Remove dead code that assumed tip == nullptr (MarcoFalke) fa2e443 refactor: Replace g_genesis_wait_cv with m_tip_block_cv (MarcoFalke) fa7f52a refactor: Use wait_for predicate to check for interrupt (MarcoFalke) 5ca28ef refactor: Split up NodeContext shutdown_signal and shutdown_request (Ryan Ofsky) fad8e7f bugfix: Mark m_tip_block_cv as guarded by m_tip_block_mutex (MarcoFalke) fa18586 refactor: Add missing GUARDED_BY(m_tip_block_mutex) (MarcoFalke) fa4c075 doc: Clarify waitTipChanged docs (MarcoFalke) Pull request description: `g_genesis_wait_cv` is similar to `m_tip_block_cv` but shuffling everything through a redundant `boost::signals2`. So remove it, along with some other dead code, as well as minor fixups. ACKs for top commit: ryanofsky: Code review ACK fa22e5c (just rebased since last review) Sjors: ACK fa22e5c TheCharlatan: ACK fa22e5c Tree-SHA512: a2cb59b651aaf85a3574723adfe403487566788ad945933b0458816ccc841fce08ca77b31afbd2d6adb5bf1deed7229c028bee74fb4bbaf6576e9edcfa0ad817
33381ea scripted-diff: Modernize nLocalServices to m_local_services (Fabian Jahr) Pull request description: The type of the `nLocalServices` variable was changed to `std::atomic<ServiceFlags>` in #30807 and I suggested the variable name to get updated with a scripted diff along with it. It wasn't included in the PR but I am still suggesting to do it as a follow-up since I had already prepared the commit. ACKs for top commit: sipa: utACK 33381ea achow101: ACK 33381ea furszy: utACK 33381ea jonatack: ACK 33381ea theStack: ACK 33381ea Tree-SHA512: 407ea9eac694f079aa5b5c1611b5874d7a0897ba6bc3aa0570be94afe1bf3a826657b6890b6597c03c063e95b9dc868f0bdfbfc41e77ec7e06f5b045bf065c71
…-declaring RemovalReasonToString ca2e4ba refactor: include the proper header rather than forward-declaring RemovalReasonToString (Cory Fields) Pull request description: Trivial no-op fixup. This was pointed out by #31053, which causes the include order to be shuffled around: ``` [21:49:26.130] /ci_container_base/src/validationinterface.cpp:22:13: error: redundant 'RemovalReasonToString' declaration [readability-redundant-declaration,-warnings-as-errors] [21:49:26.130] 22 | std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; [21:49:26.130] | ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [21:49:26.130] /ci_container_base/src/kernel/mempool_removal_reason.h:22:13: note: previously declared here [21:49:26.130] 22 | std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; [21:49:26.130] | ^ ``` I don't see any reason why the include shouldn't just be used. ACKs for top commit: maflcko: lgtm ACK ca2e4ba hebasto: ACK ca2e4ba, IWYU seems [agree](https://cirrus-ci.com/task/6170839912022016): TheCharlatan: ACK ca2e4ba Tree-SHA512: e3584cae4f50bf2bc6c824bfaddfe683ef6a17d16138d0cbcc544b98bd64d5d7353b0826b1e8cf16e12410e27b0fcedde27100d4241b7cc194cd4465c8175a5b
36a6d4b doc: update IBD requirements in doc/README.md (Mackain) Pull request description: A small change to the first paragraph of the Setup part of the README that has been bugging me for a while. The disk space required for the Bitcoin transactions can no longer be described as "a few" hundred gigabytes. So I thought it was time it was changed to "several" instead. <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md --> <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. --> <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. --> ACKs for top commit: achow101: ACK 36a6d4b danielabrozzoni: ACK 36a6d4b jonatack: ACK 36a6d4b ismaelsadeeq: ACK 36a6d4b tdb3: ACK 36a6d4b itornaza: ACK 36a6d4b Tree-SHA512: c5b21aca526c0ebe5f3234bd72e4080dc64cbba0ccd2306397aafe8349bc3573773ee64ff31fafcf59ea1afc7527caaf6d7cd8fe798311d9dc11ad0cd539e21e
…ution() 525e9dc Add submitSolution to BlockTemplate interface (Sjors Provoost) 47b4875 Add getCoinbaseMerklePath() to Mining interface (Sjors Provoost) 63d6ad7 Move BlockMerkleBranch back to merkle.{h,cpp} (Sjors Provoost) Pull request description: The new `BlockTemplate` interface introduced in #30440 allows for a more efficient way for a miner to submit the block solution. Instead of having the send the full block, it only needs to provide the nonce, timestamp, version fields and coinbase transaction. This PR introduces `submitSolution()` for that. It's currently unused. #29432 and Sjors#48 use it to process the Stratum v2 message [SubmitSolution](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#77-submitsolution-client---server). The method should be sufficiently generic to work with alternative mining protocols (none exist that I'm aware off). This PR also introduces `getCoinbaseMerklePath()`, which is needed in Stratum v2 to construct the `merkle_path` field of the `NewTemplate` message (see [spec](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#72-newtemplate-server---client)). The coinbase merkle path is also used in Stratum "v1", see e.g. https://bitcoin.stackexchange.com/questions/109820/questions-on-merkle-root-hashing-for-stratum-pools This last function uses `BlockMerkleBranch` which was moved to the test code in #13191. The reason back then for moving it was that it was no longer used. This PR moves it back. This PR does not change behaviour since both methods are unused. ACKs for top commit: achow101: ACK 525e9dc itornaza: Code review ACK 525e9dc tdb3: Code review and light test ACK 525e9dc ryanofsky: Code review ACK 525e9dc. Left minor suggestions but none are important, and looks like this could be merged as-is Tree-SHA512: 2a6a8f5d409ff4926643193cb67702240c7c687615414371e53383d2c13c485807f65e21e8ed98515b5456eca3d9fca13cec04675814a4081467d88b849c5653
…tcoin-build-config.h Follow-up for PR #30856, commit 0dd6625. -BEGIN VERIFY SCRIPT- sed -i "s|config/bitcoin-config\.h|bitcoin-build-config.h|g" $(git grep -l config/bitcoin-config\.h) sed -i "s|bitcoin-config\.h|bitcoin-build-config.h|g" $(git grep -l "bitcoin-config\.h" ./src ./test ./cmake) git mv ./cmake/bitcoin-config.h.in ./cmake/bitcoin-build-config.h.in -END VERIFY SCRIPT-
…onfig.h 882f736 doc: lint: correct outdated comment (s/Makefile.am/CMakeLists.txt/) (Sebastian Falbesoner) 1786be7 scripted-diff: drop config/ subdir for bitcoin-config.h, rename to bitcoin-build-config.h (Sebastian Falbesoner) Pull request description: This PR is a follow-up to #30856, as suggested in comment #30856 (comment). With the scripted diff, review should be fairly trivial, but it could still be seen as controversial due to the large number of files (78 in total) being touched. ACKs for top commit: fanquake: ACK 882f736 Tree-SHA512: 2e6cae4590f660e741edf84df456168b8b1f3861d381cfebf6647bb0a303c26bf7b969a837e0058e59bf852d220990dd8f5f400d8975fd0fab106d0507a70c9b
0b3ec8c clusterlin: remove Cluster type (Pieter Wuille) 1c24c62 clusterlin: merge two DepGraph fuzz tests into simulation test (Pieter Wuille) 0606e66 clusterlin: add DepGraph::RemoveTransactions and support for holes in DepGraph (Pieter Wuille) 75b5d42 clusterlin: make DepGraph::AddDependency support multiple dependencies at once (Pieter Wuille) abf5064 clusterlin: simplify DepGraphFormatter::Ser (Pieter Wuille) eaab55f clusterlin: rework DepGraphFormatter::Unser (Pieter Wuille) 5901cf7 clusterlin: abstract out DepGraph::GetReduced{Parents,Children} (Pieter Wuille) Pull request description: Part of cluster mempool: #30289 This adds: * `DepGraph::AddDependencies` to add 0 or more dependencies to a single transaction at once (identical to calling `DepGraph::AddDependency` once for each, but more efficient). * `DepGraph::RemoveTransactions` to remove 0 or more transactions from a depgraph. * `DepGraph::GetReducedParents` (and `DepGraph::GetReducedChildren`) to get the (reduced) direct parents and children of a transaction in a depgraph. After which, the `Cluster` type is removed. This is the result of fleshing out the design for the "intermediate layer" ("TxGraph", no PR yet) between the cluster linearization layer and the mempool layer. My earlier thinking was that TxGraph would store `Cluster` objects (vectors of pairs of `FeeFrac`s and sets of parents), and convert them to `DepGraph` on the fly whenever needed. However, after more consideration, it seems better to have TxGraph store `DepGraph` objects, and manipulate them directly without constantly re-creating them. This requires `DepGraph` to have some additional functionality. The bulk of the complexity here is the addition of `DepGraph::RemoveTransactions`, which leaves the remaining transactions' positions within the `DepGraph` untouched (we want existing identifiers to remain valid), so this implies that graphs can now have "holes" (positions that are unused, but followed by positions that are used). To enable that, an extension of the fuzz/test serialization format `DepGraphFormatter` is included to deal with such holes. ACKs for top commit: sdaftuar: reACK 0b3ec8c instagibbs: reACK 0b3ec8c ismaelsadeeq: reACK 0b3ec8c glozow: ACK 0b3ec8c, reviewed range-diff from aab53dd and `clusterlin_depgraph_sim` Tree-SHA512: a804b7f26d544c5cb0847322e235c810525cb0607737be6116c3156d582da3ba3352af8ea48e74eed5268f9c3eca63b30181d01b23a6dd0be1b99191f81cceb0
ccd10fd build: Add missing USDT header dependency to kernel (Cory Fields) Pull request description: Noticed while testing a branch that replaces `boost::multi_index` with a custom replacement. Currently depends builds pick up usdt and boost from the same path, and because boost always exists, the usdt path is implicitly included. So without boost, USDT isn't found. An alternative to this would be to disable USDT for the kernel. I'd be open to either approach. ACKs for top commit: hebasto: ACK ccd10fd, the diff looks correct. fanquake: ACK ccd10fd Tree-SHA512: 2f91b8d5c8b169f7b72323d9163b5201f606ccdab95de7085847d2a672d10f940f69642c2528226a5efa4c589af24ca3bb9dd909eed0993e4cecd9689b4bed2f
Closes: #25055 Add doxygen comment to the m_args member in the unit test framework, clarifying its purpose.
Running Bitcoin Core on unsupported OSes may expose users to security issues. macOS Monterey 12 received its final security update (12.7.6) on July 2024. Apple classifies the hardware that can run macOS 12 at most as "obsolete worldwide".
1fe1b3b doc: doxygen comment for m_args usage in tests (willcl-ark) Pull request description: Closes: #25055 Add a doxygen comment to the `m_args` member in the unit test framework, clarifying its purpose and providing usage guidelines. ACKs for top commit: maflcko: lgtm ACK 1fe1b3b brunoerg: ACK 1fe1b3b Tree-SHA512: 9b8dc30e3b0d26c0cecec4599dc5addca519965603073d02f37fa0a46c488659958e327d9c25da8acdb4bb9b082a64455baaffb406ac11827d7f56a094522fce
e64b2f1 doc: cmake: prepend and explain "build/" where needed (Larry Ruane) Pull request description: This is a small follow-on to #30741, prepend `build/` to the path for `test_runner.py`. ACKs for top commit: jonatack: ACK e64b2f1 maflcko: lgtm ACK e64b2f1 tdb3: re ACK e64b2f1 Tree-SHA512: 80943d4f342987bf060adacb1c7db2e9ff8de5a6da592846ba23f230281d3a5b306162c4c86e61739a29323eaa4abf09f69f41302996d5809f448e5788a74a87
d823ba6 doc: fuzz: remove Honggfuzz NetDriver instructions (brunoerg) Pull request description: Remove Honggfuzz NetDriver instructions from the documentation since it has not been useful for us. See #30957 and #31012. ACKs for top commit: maflcko: lgtm ACK d823ba6 marcofleon: ACK d823ba6 Tree-SHA512: f63fde1076d523dc5e511ef868ca3c1ea2e38fe7df56ae275f33209581f96452d86effedb54d9b0ee8b7a1d492b610799807a727d8bd81e2286d31db4aa68731
a0e089a build: Bump minimum supported macOS to 13.0 (Hennadii Stepanov) Pull request description: Running Bitcoin Core on unsupported OSes may expose users to security issues. macOS Monterey 12 received its final security update ([12.7.6](https://support.apple.com/en-us/100100)) on July 2024. Apple classifies the hardware that can run macOS 12 at most as ["obsolete worldwide"](https://support.apple.com/en-us/102772). ACKs for top commit: maflcko: lgtm ACK a0e089a m3dwards: ACK a0e089a itornaza: reACK a0e089a Tree-SHA512: b219730de87bcb2bcb40a972e910f516c739a538b0741fc245d23df04650f7e2f5774c38c1d1c9c053ed9e2a377488002feb708e8c7cba9c0070b81169719b10
fa1b139 Bump python minimum supported version to 3.10 (MarcoFalke) Pull request description: All supported operating systems ship with python 3.10 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python features. For reference: * https://packages.debian.org/bookworm/python3 * https://packages.ubuntu.com/jammy/python3 * FreeBSD 13/14 ships with 3.11 * CentOS-like 8/9 ships with 3.11/3.12 (via `appstream`) * OpenSuse Tumbleweed ships with all python versions, e.g. https://software.opensuse.org/package/python312-base This is for Bitcoin Core 29.0 in 2025 (next year), not the soon upcoming 28.0 this fall. ACKs for top commit: achow101: ACK fa1b139 AngusP: ACK fa1b139 l0rinc: ACK fa1b139 stickies-v: ACK fa1b139 Tree-SHA512: 910b202ff2374bb21c93e5249a151fd2c8f63759bed5659676b0e467afa6e8e977be797c3fccceca303c82575e11ec236a8d7c5880910e4314b3875b820e7e8a
… inputs/outputs ec585f1 Reserve space for transaction inputs in CreateTransactionInternal (Lőrinc) c76aaaf Reserve space for transaction outputs in CreateTransactionInternal (Lőrinc) Pull request description: Reserved memory for the transaction inputs and outputs. Split out of https://github.com/bitcoin/bitcoin/pull/30050/files#r1597631104 ACKs for top commit: achow101: ACK ec585f1 TheCharlatan: ACK ec585f1 stickies-v: ACK ec585f1 Tree-SHA512: de399fb19824423467f48af64aa57f41a23cdd00eb17461e0131e4deafdd15e0d2daebf6a0a7ac7728b2fb486b2a54f1a7ef26bbe823c56b2a09f892f6b9a581
This duplicates what is in depends, and is outdated.
fa43c4f test: Print CompletedProcess object on error (MarcoFalke) Pull request description: It would be good to know the output on `Error parsing command output`. Otherwise test failures are meaningless: #30792 (comment) Fix it by just printing the full `CompletedProcess` object. Also, use the modern `subprocess.run` to simplify the code. ACKs for top commit: BrandonOdiwuor: Code Review ACK fa43c4f laanwj: This contains some useful information, so ACK fa43c4f Tree-SHA512: ae7c1cb1f48af2a6feae6d1a5a967c0720f6c6675c1ce20ace7cac18c00f3d4069b8abcc58204855e92ff5303158b9a942bab3b71acae0737768d941a5773c91
79aa828 doc: drop LLVM install instructions (fanquake) Pull request description: Followup from #31048. ACKs for top commit: maflcko: lgtm ACK 79aa828 hebasto: ACK 79aa828. Tree-SHA512: 9404845cc9a17f85363ce893addadaaba839b4a37e1e3e64ad4a50eb237ffb78636970480ff2f486ff5bd1b3dba9b85bf3d6654a680b11c6832d17daf6dd6c0a
…tories a647d44 doc: update signet documentation related to build directories (Torkel Rogstad) Pull request description: While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in #30741 ACKs for top commit: maflcko: lgtm ACK a647d44 pablomartin4btc: ACK a647d44 tdb3: Code review and light test ACK a647d44 Tree-SHA512: ac7c3806e0ff65860c41d7b7bdad538368d8a6d8d289c10f9714804f963bafd3a9658301b6697f110f5462a92826b62770963508d5eebf88bf9a0a8442d9f72d
184f12c doc: remove dependency install instructions from win docs (fanquake) Pull request description: This duplicates what is in depends, and is outdated. Closes #31090. ACKs for top commit: maflcko: lgtm ACK 184f12c jarolrod: ACK 184f12c BrandonOdiwuor: ACK 184f12c Tree-SHA512: 089c9ff91c501c22ec1b9d5925a2b8c6cd1ea9ac2b75dd6a8c5fe75cf2f0090d808842cb321017894d2da70a30a87dbc1c4c481771d3c4aba13ce44244fcf392
…noseconds cd0edf2 tracing: cast block_connected duration to nanoseconds (0xb10c) Pull request description: When the `validation:block_connected` tracepoint was introduced in 8f37f5c, the connect block duration was passed in microseconds `µs`. By starting to use steady clock in fabf1cd this changed to nanoseconds `ns`. As the test only checked if the duration value is `> 0` as a plausibility check, this went unnoticed. This was detected this when setting up monitoring for block validation time as part of the Great Consensus Cleanup Revival discussion. This change casts the duration explicitly to nanoseconds, updates the documentation, and adds a check for an upper bound to the tracepoint interface tests. The upper bound is quite lax as mining the block takes much longer than connecting the empty test block. It's however able to detect a duration passed in an incorrect unit (1000x off). A previous version of this PR casted the duration to microseconds `µs` - however, as the last three major releases have had the duration as nanoseconds (and this went unnoticed), we assume that this is the API now and changeing it back to microseconds would break the API again. See also #29877 (comment) ACKs for top commit: maflcko: re-lgtm ACK cd0edf2 laanwj: re-ACK cd0edf2 Tree-SHA512: 54a1eea0297e01c07c2d071ffafbf97dbd080f763e1dc0014ff086a913b739637c1634b1cf87c90b94a3c2f66006acfaada0414a15769cac761e03bc4aab2a77
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.