aboutsummaryrefslogtreecommitdiff
path: root/src/node
AgeCommit message (Collapse)Author
2023-04-19move-only: Extract common/args and common/config.cpp from util/systemTheCharlatan
This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.
2023-04-03Merge bitcoin/bitcoin#27254: refactor: Extract util/fs from util/systemfanquake
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan) 106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan) b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan) 18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR #25152. #### Context There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/27238. #### Changes Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files. There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory. Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed. ACKs for top commit: hebasto: ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3 Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
2023-03-26clang-tidy: Add `performance-inefficient-vector-operation` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html
2023-03-23refactor: Move fs.* to util/fs.*TheCharlatan
The fs.* files are already part of the libbitcoin_util library. With the introduction of the fs_helpers.* it makes sense to move fs.* into the util/ directory as well.
2023-03-23Add missing fs.h includesTheCharlatan
The inclusion of this header should not depend on the inclusion of other headers that include fs.h themselves.
2023-03-16Merge bitcoin/bitcoin#26177: refactor / kernel: Move non-gArgs chainparams ↡fanquake
functionality to kernel b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6 refactor: Don't use global chainparams in chainstatemanager method (TheCharlatan) 382b692a503355df7347efd9c128aff465b5583e Split non/kernel chainparams (Carl Dong) edabbc78a3bc272b2b802e1dbab73d6ed8e31e96 Add factory functions for Main/Test/Sig/Reg chainparams (Carl Dong) d938098398814f37fed9b018b44716179cfa4b03 Remove UpdateVersionBitsParameters (Carl Dong) 84b85786f0f5cb23cc257a4464ae345e1d372313 Decouple RegTestChainParams from ArgsManager (Carl Dong) 76cd4e7c96242398172989609f1b9a8843c404b4 Decouple SigNetChainParams from ArgsManager (Carl Dong) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". dongcarl is the original author of this patchset, these commits were taken from https://github.com/dongcarl/bitcoin/tree/2022-03-libbitcoinkernel-chainparams-args-only. #### Context The bitcoin kernel library currently relies on code containing user configurations through the `ArgsManager`. This is not optimal, since as a stand-alone library it should not rely on bitcoind's argument parsing logic. Instead, its interfaces should accept control and options structs that control the kernel library's desired configuration. Similar work towards decoupling the `ArgsManager` from the kernel has been done in https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527 and https://github.com/bitcoin/bitcoin/pull/25862. #### Changes By moving the `CChainParams` class definition into the kernel and giving it new factory functions `CChainParams::{RegTest,SigNet,Main,TestNet}`it can be constructed without an `ArgsManager` reference, unlike the current factory function `CreateChainParams`. The first few commits remove uses of `ArgsManager` within `CChainParams`. Then the `CChainParams` definition is moved to a new file in the `kernel/` subdirectory. ACKs for top commit: MarcoFalke: re-ACK b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6 πŸ› ryanofsky: Code review ACK b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6. Only changes since last review were recent review suggestions. ajtowns: ACK b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6 Tree-SHA512: 3835aca1d3e3c75cc3303dd584bab3a77e58f6c678724a5e359fe4b0e17e0763a00931ee6191f516b9fde50496f59cc691f0709c0254206db3863bbf7ab2cacd
2023-03-15Split non/kernel chainparamsCarl Dong
Moves chainparams code not using the ArgsManager to the kernel. Subsequently use the kernel chainparams header now where possible in order to further decouple chainparams call sites from gArgs.
2023-03-15refactor: Add and use PRUNE_TARGET_MANUAL constexprMarcoFalke
2023-03-15Move ::fImporting to BlockManagerMarcoFalke
2023-03-15Pass fImporting to ImportingNow helper classMarcoFalke
2023-03-15Move ::fPruneMode into BlockManagerMarcoFalke
2023-03-15Move ::nPruneTarget into BlockManagerMarcoFalke
2023-03-14Merge bitcoin/bitcoin#27238: refactor: Split logging utilities from system.hfanquake
aaced5633b81b2f08b993106a527e2a0e1d663c1 refactor: Move error() from util/system.h to logging.h (Ben Woosley) e7333b420e35054d9302f9c58fd47b6152cbd35f refactor: Extract util/exception from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". These commits were originally authored by empact and are taken from their parent PR #25152. #### Context There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. #### Changes Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving some logging functions out of the `system.*` files. Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed. ACKs for top commit: MarcoFalke: re-ACK aaced5633b81b2f08b993106a527e2a0e1d663c1 🐍 Tree-SHA512: cb39f4cb7a77e7dc1887b1cbf340d53decab8880fc00878a2f12dc627fe67245b4aafd4cc31a9eab0fad1e5bb5d0eb4cdb8d501323ca200fa6ab7b201ae34aea
2023-03-13refactor: Move error() from util/system.h to logging.hBen Woosley
error is a low-level function with a sole dependency on LogPrintf, which is defined in logging.h The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager defined in system.h. Moving the function out of system.h allows including it from a separate source file without including the ArgsManager definitions from system.h.
2023-03-10refactor: Consistently use context args over gArgs in node/interfacesMarcoFalke
2023-03-08Merge bitcoin/bitcoin#27189: util: Use steady clock in SeedStrengthen, ↡fanquake
FindBestImplementation, FlushStateToDisk fa1b4e5c3294fc9aec033892a4a4d7b5cfc015aa Use steady clock in FlushStateToDisk (MarcoFalke) 1111e2f8b43cd9ed62dcf6b571a224b84fc421fd Use steady clock in SeedStrengthen and FindBestImplementation (MarcoFalke) Pull request description: There may be a theoretical deadlock for the duration of the offset when the system clock is adjusted into a past time while executing `SeedStrengthen`. Fix this by using steady clock. Do the same in `FindBestImplementation`, which shouldn't be affected, because it discards outlier measurements. However, doing the same there for consistency seems fine. Do the same in `FlushStateToDisk`, which should make the flushes more steady, if the system clock is adjusted by a large offset. ACKs for top commit: john-moffett: ACK fa1b4e5c3294fc9aec033892a4a4d7b5cfc015aa willcl-ark: ACK fa1b4e5c3 Tree-SHA512: cc625e796b186accd53222bd64eb57d0512bc7e588312d254349b542bbc5e5daac348ff2b3b3f7dc5ae0bbbae2ec11fdbf3022cf2164211633765a4b0108e83e
2023-03-07Merge bitcoin/bitcoin#25740: assumeutxo: background validation completionAndrew Chow
2b373fe49d64f04ceab2309d3f40da7bac6b37d6 docs: update assumeutxo.md (James O'Beirne) 87a1108c81fe0cb15c3860e3a67dc1f43ffec705 test: add snapshot completion unittests (James O'Beirne) d70919a88fc90a2662f9a844deb085d03ee7b5d8 refactor: make MempoolMutex() public (James O'Beirne) 7300ced9de22e6d1bff816e6538d3370cebe7501 log: add LoadBlockIndex() message for assumedvalid blocks (James O'Beirne) d96c59cc5cd2f73f1f55c133c52208671fe75ef3 validation: add ChainMan logic for completing UTXO snapshot validation (James O'Beirne) f2a4f3376f1476b38a79a549bd81ba3006225df6 move-only-ish: init: factor out chainstate initialization (James O'Beirne) 637a90b973f60555ea4fef4b845ffa7533dcb866 add Chainstate::HasCoinsViews() (James O'Beirne) c29f26b47b8ef978d8689dc0222aa663361ee6cb validation: add CChainState::m_disabled and ChainMan::isUsable (James O'Beirne) 5ee22cdafd2562bcb8bf0ae6025e4b53c826382d add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}() (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) Part two of replacing https://github.com/bitcoin/bitcoin/pull/24232. --- When a user activates a snapshot, the serialized UTXO set data is used to create an "assumed-valid" chainstate, which becomes active in an attempt to get the node to network tip as quickly as possible. Simultaneously in the background, the already-existing chainstate continues "conventional" IBD to both accumulate full block data and serve as a belt-and-suspenders to validate the assumed-valid chainstate. Once the background chainstate's tip reaches the base block of the snapshot used, we set `m_stop_use` on that chainstate and immediately take the hash of its UTXO set; we verify that this matches the assumeutxo value in the source code. Note that while we ultimately want to remove this background chainstate, we don't do so until the following initialization process, when we again check the UTXO set hash of the background chainstate, and if it continues to match, we remove the (now unnecessary) background chainstate, and move the (previously) assumed-valid chainstate into its place. We then reinitialize the chainstate in the normal way. As noted in previous comments, we could do the filesystem operations "inline" immediately when the background validation completes, but that's basically just an optimization that saves disk space until the next restart. It didn't strike me as worth the risk of moving chainstate data around on disk during runtime of the node, though maybe my concerns are overblown. The final result of this completion process is a fully-validated chain, where the only evidence that the user synced using assumeutxo is the existence of a `base_blockhash` file in the `chainstate` directory. ACKs for top commit: achow101: ACK 2b373fe49d64f04ceab2309d3f40da7bac6b37d6 Tree-SHA512: a204e1d6e6932dd83c799af3606b01a9faf893f04e9ee1a36d63f2f1ccfa9118bdc1c107d86976aa0312814267e6a42074bf3e2bf1dead4b2513efc6d955e13d
2023-03-07validation: add ChainMan logic for completing UTXO snapshot validationJames O'Beirne
Trigger completion when a background validation chainstate reaches the same height as a UTXO snapshot, and handle cleaning up the chainstate on subsequent startup.
2023-03-02Use steady clock in SeedStrengthen and FindBestImplementationMarcoFalke
2023-02-28Add InitError(error, details) overloadRyan Ofsky
This is only used in the current PR to avoid ugly `strprintf(Untranslated("%s:\n%s"), str, MakeUnorderedList(details)` boilerplate in init code. But in the future the function could be extended and more widely used to include more details in GUI error messages or display them in a more readable way, see code comment.
2023-02-28Merge bitcoin/bitcoin#26533: prune: scan and unlink already pruned block ↡Andrew Chow
files on startup 3141eab9c669488a2e7fef5f60d356ac92294922 test: add functional test for ScanAndUnlinkAlreadyPrunedFiles (Andrew Toth) e252909e561e47d75cb3a892657662a139f6532c test: add unit test for ScanAndUnlinkAlreadyPrunedFiles (Andrew Toth) 77557dda4a123515d0fa2a545ee21d7c43a66988 prune: scan and unlink already pruned block files on startup (Andrew Toth) Pull request description: There are a few cases where we can mark a block and undo file as pruned in our block index, but not actually remove the files from disk. 1. If we call `FindFilesToPrune` or `FindFilesToPruneManual` and crash before `UnlinkPrunedFiles`. 2. If on Windows there is an open file handle to the file somewhere else when calling `fs::remove` in `UnlinkPrunedFiles` (https://en.cppreference.com/w/cpp/filesystem/remove, https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-deletefilew#remarks). This could be from another process, or if we are calling `ReadBlockFromDisk`/`ReadRawBlockFromDisk` without having a lock on `cs_main` (which has been allowed since https://github.com/bitcoin/bitcoin/commit/ccd8ef65f93ed82a87cee634660bed3ac17d9eb5). This PR mitigates this by scanning all pruned block files on startup after `LoadBlockIndexDB` and unlinking them again. ACKs for top commit: achow101: ACK 3141eab9c669488a2e7fef5f60d356ac92294922 pablomartin4btc: re-ACK with added functional test 3141eab9c669488a2e7fef5f60d356ac92294922. furszy: Code review ACK 3141eab9 theStack: Code-review ACK 3141eab9c669488a2e7fef5f60d356ac92294922 Tree-SHA512: 6c73bc57838ad1b7e5d441af3c4d6bf4c61c4382e2b86485e57fbb74a61240710c0ceeceb8b4834e610ecfa3175c6955c81ea4b2285fee11ca6383f472979d8d
2023-02-24doc: add explanation for fail_on_insufficient_dbcacheRyan Ofsky
2023-02-24init: Return more fitting ChainStateLoadStatus if verification was interruptedMartin Zumsande
This also avoids a misleading block index loadtime log entry in init. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-02-22Merge bitcoin/bitcoin#25574: validation: Improve error handling when ↡Andrew Chow
VerifyDB dosn't finish successfully 0af16e7134459e0820ab95d751093876c1ec4c6d doc: add release note for #25574 (Martin Zumsande) 57ef2a4812f443b2d734f43cebf3ef5038da83f2 validation: report if pruning prevents completion of verification (Martin Zumsande) 0c7785bb2540b69564104767d38342704230cbc2 init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache (Martin Zumsande) d6f781f1cfcbc2c2ad5ee289a0642ed00386d013 validation: return VerifyDBResult::INTERRUPTED if verification was interrupted (Martin Zumsande) 6360b5302d2675788de5c4a28ea77d823f6d809e validation: Change return value of VerifyDB to enum type (Martin Zumsande) Pull request description: `VerifyDB()` can fail to complete due to insufficient dbcache at the level 3 checks. This PR improves the error handling in this case in the following ways: - The rpc `-verifychain` now returns false if the check can't be completed due to insufficient cache - During init, we only log a warning if the default values for `-checkblocks` and `-checklevel` are taken and the check doesn't complete. However, if the user actively specifies one of these args, we return with an InitError if we can't complete the check. This PR also changes `-verifychain` RPC to return `false` if the verification didn't finish due to missing block data (pruning) or due to being interrupted by the node being shutdown. Previously, this PR also included a fix for a possible assert during verification - this was done in #27009 (now merged). ACKs for top commit: achow101: ACK 0af16e7134459e0820ab95d751093876c1ec4c6d ryanofsky: Code review ACK 0af16e7134459e0820ab95d751093876c1ec4c6d. Only small suggested changes since the last review, like renaming some of the enum values. I did leave more suggestions, but they are not very important and could be followups john-moffett: ACK 0af16e7134459e0820ab95d751093876c1ec4c6d MarcoFalke: lgtm re-ACK 0af16e7134459e0820ab95d751093876c1ec4c6d 🎚 Tree-SHA512: 84b4f767cf9bfbafef362312757c9bf765b41ae3977f4ece840e40c52a2266b1457832df0cdf70440be0aac2168d9b58fc817238630b0b6812f3836ca950bc0e
2023-02-22move-only-ish: init: factor out chainstate initializationJames O'Beirne
Moves chainstate initialization into its own function. This is necessary to later support a more readable way of handling background-validation chainstate cleanup during init, since the chainstate initialization functions may need to be repeated after moving leveldb filesystem content around. This commit isn't strictly necessary, but the alternative is to (ab)use the `while` loop in init.cpp with a `continue` on the basis of a specific ChainstateLoadingError return value from LoadChainstate. Not only is this harder to read, but it can't be unittested. The approach here lets us consolidate background-validation cleanup to LoadChainstate, and therefore exercise it within tests. This commit is most easily reviewed with git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-space-change
2023-02-20Merge bitcoin/bitcoin#26883: src/node/miner cleanups, follow-ups for #26695glozow
6a5e88e5cf06a6b410486cc36aba7afece0d9da9 miner: don't re-apply default Options value if argument is unset (stickies-v) ea72c3d9d594b2ea9b3397e64efd08f8563cb400 refactor: avoid duplicating BlockAssembler::Options members (stickies-v) cba749a9b7a6cd24e8887bddeb0430a1ebc783da refactor: rename local gArgs to args (stickies-v) Pull request description: Two follow-ups for #26695, both refactoring and no observed (*) behaviour change: - Rename `gArgs` to `args` because it's not actually a global - Add `BlockAssembler::Options` as a (private) member to `BlockAssembler` to avoid having to assign all the options individually, essentially duplicating them Reduces LoC and makes the code more readable, in my opinion. --- (*) as [pointed out by ajtowns](https://github.com/bitcoin/bitcoin/pull/26883#discussion_r1068247937), this PR changes the interface of `ApplyArgsManOptions()`, making this not a pure refactoring PR. In practice, `ApplyArgsManOptions()` is never called in such a way that this leads to observed behaviour change. Regardless, I've carved out the potential behaviour change into a separate commit and would be okay with dropping it, should it turn out to be controversial. ACKs for top commit: glozow: ACK 6a5e88e5cf TheCharlatan: Light code review ACK 6a5e88e5cf06a6b410486cc36aba7afece0d9da9 Tree-SHA512: 15c30442ff0e070b1a58dc4c9615550d619ce35b4a2596b2c0a9d790259bbf987cab708f7cbb1057a8cf8b4c3226f3ad981282d3499ac442094806492a5f68ce
2023-02-16validation: report if pruning prevents completion of verificationMartin Zumsande
Now the verifychain RPC returns false if the checks didn't finish because the blocks requested to be queried have been pruned.
2023-02-16init, validation: Improve handling if VerifyDB() fails due to insufficient ↡Martin Zumsande
dbcache The rpc command verifychain now fails if the dbcache was not sufficient to complete the verification at the specified level and depth. In the same situation, the VerifyDB check during Init will now fail (and lead to an early shutdown) if the user has explicitly specified -checkblocks or -checklevel but the check couldn't be executed because of the limited cache. If the user didn't change any of the two and is using the defaults, log a warning but don't prevent the node from starting up.
2023-02-16validation: return VerifyDBResult::INTERRUPTED if verification was interruptedMartin Zumsande
This means that the -verifydb RPC will now return false if it cannot finish due to the node being shutdown.
2023-02-16validation: Change return value of VerifyDB to enum typeMartin Zumsande
This does not change behavior. It is in preparation for special handling of the case where VerifyDB doesn't finish for various reasons, but doesn't fail.
2023-02-10refactor, validation: Add ChainstateManagerOpts db optionsRyan Ofsky
Use ChainstateManagerOpts struct to remove ArgsManager uses from validation.cpp. This commit does not change behavior.
2023-02-10refactor, txdb: Use DBParams struct in CBlockTreeDBRyan Ofsky
Use DBParams struct to remove ArgsManager uses from txdb. To reduce size of this commit, this moves references to gArgs variable out of txdb.cpp to calling code in chainstate.cpp. But these moves are temporary. The gArgs references in chainstate.cpp are moved out to calling code in init.cpp in later commits. This commit does not change behavior.
2023-02-10refactor, txdb: Add CoinsViewOptions structRyan Ofsky
Add CoinsViewOptions struct to remove ArgsManager uses from txdb. To reduce size of this commit, this moves references to gArgs variable out of txdb.cpp to calling code in validation.cpp. But these moves are temporary. The gArgs references in validation.cpp are moved out to calling code in init.cpp in later commits. This commit does not change behavior.
2023-02-10refactor, dbwrapper: Add DBParams and DBOptions structsRyan Ofsky
Add DBParams and DBOptions structs to remove ArgsManager uses from dbwrapper. To reduce size of this commit, this moves references to gArgs variable out of dbwrapper.cpp to calling code in txdb.cpp. But these moves are temporary. The gArgs references in txdb.cpp are moved out to calling code in init.cpp in later commits. This commit does not change behavior.
2023-02-07Remove reindex special case from the progress bar labelMarcoFalke
2023-01-30Merge bitcoin/bitcoin#26965: refactor: Remove stray cs_main redundant ↡fanquake
declaration faba08b5b4dd5dedb457db18696de6e15839c696 refactor: Remove stray cs_main redundant declaration (MarcoFalke) fa02591edff0255c5120b5acb2366542a61c251f doc: Export threadsafety.h from sync.h (MarcoFalke) Pull request description: Looks like this was forgotten when introducing kernel/cs_main ? Also, there is a commit to export threadsafety.h from sync.h. ACKs for top commit: hebasto: ACK faba08b5b4dd5dedb457db18696de6e15839c696 Tree-SHA512: 0aa58e7693b6fcd504f9da7339f8baa463a6407f67b27f68002db705f4642321ac3765f16c3d906c925ee24085591b79160a62fa5f4aaf6f2e5dcc788411800d
2023-01-30Merge bitcoin/bitcoin#26649: refactor: Use AutoFile and HashVerifier ↡fanquake
(without ser-type and ser-version) where possible eeee61065fe165dcce9625f7cc4cfce9e432aafa Use AutoFile and HashVerifier where possible (MarcoFalke) fa961141f7fc515fbb6fcb9d8417108034b256ce Add HashVerifier (MarcoFalke) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `AutoFile` and `HashVerifier`. `CAutoFile` and `CHashVerifier` remain in places where it is not yet possible. ACKs for top commit: stickies-v: ACK eeee61065fe165dcce9625f7cc4cfce9e432aafa Tree-SHA512: 93786778c309ecfdc1ed43552d24ff9d966954d69a47f66faaa6de24daacd25c651f3f62bde5abbb362700298fb3c04ffbd3207a0dd13d0bd8bff7fd6d07dcf8
2023-01-25refactor: Remove stray cs_main redundant declarationMarcoFalke
2023-01-16miner: don't re-apply default Options value if argument is unsetstickies-v
ApplyArgsManOptions does not need to set default values for missing arguments, these are already defined in the BlockAssembler::Options. This commit changes the interface of ApplyArgsManOptions(). If ApplyArgsManOptions() is called again after a option is changed, this option will no longer be reset to the default value. There is no observed behaviour change due to how ApplyArgsManOptions() is currently used, and the new interface is consistent with e.g. ValidationCacheSizes and MemPoolLimits.
2023-01-16refactor: avoid duplicating BlockAssembler::Options membersstickies-v
Add Options as a member to BlockAssembler to avoid having to assign all the options individually. Additionally brings the struct more in line with how we typically define default and ArgManager values, as e.g. with ChainstateManager::Options and and CTxMemPool::Options
2023-01-16Add BlockManager::IsPruneMode()MarcoFalke
2023-01-16Add BlockManager::GetPruneTarget()MarcoFalke
2023-01-16Add BlockManager::LoadingBlocks()MarcoFalke
2023-01-16Merge bitcoin/bitcoin#26251: refactor: add kernel/cs_main.hMarcoFalke
282019cd3ddb060db350654e6f855f7ea37e0d34 refactor: add kernel/cs_main.* (fanquake) Pull request description: One place to find / include `cs_main`. No more: > // Actually declared in validation.cpp; can't include because of circular dependency. > extern RecursiveMutex cs_main; Ultimately, no more need to include `validation.h` (which also includes (heavy/boost filled) `txmempool.h`) everywhere for `cs_main`. See #26087 for another example of why that is useful. ACKs for top commit: ajtowns: ACK 282019cd3ddb060db350654e6f855f7ea37e0d34 Tree-SHA512: 142835b794873e7a09c3246d6101843ae81ec0c6295e6873130c98a2abfa5f7282748d0f1a37237a779cc71c3bc0a75d03b20313ef5398c83d4814215cbc8287
2023-01-12refactor: rename local gArgs to argsstickies-v
Avoid confusion with the global gArgs
2023-01-11Merge bitcoin/bitcoin#26695: bench: BlockAssembler on a mempool with packagesAndrew Chow
04528054fcde61aa00e009dbbe1ac350ca1cf748 [bench] BlockAssembler with mempool packages (glozow) 6ce265acf4ff6ee5057b46bcb8b55abc4422e6f8 [test util] lock cs_main before pool.cs in PopulateMempool (glozow) 8791410662ce3ab7ba6bbe9813c55369edd6e4c9 [test util] randomize fee in PopulateMempool (glozow) cba5934eb697aedbe1966ebc2817ab87232a1b59 [miner] allow bypassing TestBlockValidity (glozow) c0588523083c9c78770b8b19a52a919db56250d9 [refactor] parameterize BlockAssembler::Options in PrepareBlock (glozow) a2de971ba1c588488dde653a76853666429d4911 [refactor] add helper to apply ArgsManager to BlockAssembler::Options (glozow) Pull request description: Performance of block template building matters as miners likely want to be able to start mining on a block with transactions asap after a block is found. We would want to know if a mempool PR accidentally caused, for example, a 100x slowdown. An `AssembleBlock()` bench exists, but it operates on a mempool with 101 transactions, each with 0 ancestors or descendants and with the same fee. Adding a bench with a more complex mempool is useful because (1) it's more realistic (2) updating packages can potentially cause the algorithm to take a long time. ACKs for top commit: kevkevinpal: Tested ACK [0452805](https://github.com/bitcoin/bitcoin/pull/26695/commits/04528054fcde61aa00e009dbbe1ac350ca1cf748) achow101: ACK 04528054fcde61aa00e009dbbe1ac350ca1cf748 stickies-v: ACK 04528054f Tree-SHA512: 38c138d6a75616651f9b1faf4e3a1cd833437a486f4e84308fbee958e8462bb570582c88f7ba7ab99d80191e97855ac2cf27c43cc21585d3e4b0e227effe2fb5
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-01-04Merge bitcoin/bitcoin#26752: wallet: Remove `mempool_sequence` from ↡glozow
interface methods 55696a0ac30bcfbd555f71cbc8eac23b725f7dcf wallet: remove `mempool_sequence` from `transactionRemovedFromMempool` (w0xlt) bf19069c53501231a2f3ba59afa067913ec4d3b2 wallet: remove `mempool_sequence` from `transactionAddedToMempool` (w0xlt) Pull request description: This PR removes `mempool_sequence` from `transactionRemovedFromMempool` and `transactionAddedToMempool`. `mempool_sequence` is not used in these methods, only in ZMQ notifications. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/26752/commits/55696a0ac30bcfbd555f71cbc8eac23b725f7dcf Tree-SHA512: 621e89230bcb6edfed83e2758601a2b093822fc2dc4e9bfb00487e340f2bc4c5ac3bf6df3ca00b7fe55bb3df15858820f2bf698f403d2e48b915dd9eb47b63e0
2023-01-03Merge bitcoin/bitcoin#26289: Use util::Result in for calculating mempool ↡Andrew Chow
ancestors 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v) 5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v) f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v) 66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v) Pull request description: Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear. ## Unexpected behaviour This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs. In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit. ## Improvements ### Return value instead of out-parameters This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit. ### Observability There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](https://github.com/bitcoin/bitcoin/blob/69b10212ea5370606c7a5aa500a70c36b4cbb58f/src/node/miner.cpp#L399). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here. ACKs for top commit: achow101: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26289/commits/47c4b1f52ab8d95d7deef83050bad49d1e3e5990 glozow: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 furszy: light code review ACK 47c4b1f5 aureleoules: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
2023-01-03Use AutoFile and HashVerifier where possibleMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-24 14:35:12 +0000