[kernel 3c/n] Decouple validation cache initialization from ArgsManager
#25527
Conversation
This file contains hidden or 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
|
So it seems like the static asserts I added revealed that on |
3168fbf to
7f443a0
Compare
|
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
7f443a0 to
1bcae96
Compare
This was referenced Jul 2, 2022
Merged
1bcae96 to
8e40757
Compare
8e40757 to
7dfe0c2
Compare
7dfe0c2 to
66767ab
Compare
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Aug 4, 2022
|
I just realized I asked to rename At least it didn't go in a circle, though |
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 17, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from db_wrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 17, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from db_wrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 17, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 17, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 17, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 18, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 22, 2022
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 22, 2022
Code in the libbitcoin_kernel library should not be calling ArgsManager methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager calls from kernel modules. This PR does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 26, 2022
Code in the libbitcoin_kernel library should not be calling ArgsManager methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager calls from kernel modules. This PR does not change behavior in any way.
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Aug 26, 2022
Code in the libbitcoin_kernel library should not be calling ArgsManager methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager calls from kernel modules. This PR does not change behavior in any way.
This was referenced Sep 24, 2022
ryanofsky
added a commit
to ryanofsky/bitcoin
that referenced
this pull request
Oct 14, 2022
Code in the libbitcoin_kernel library should not be calling ArgsManager methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other ArgsManager calls from kernel modules. This PR does not change behavior in any way.
achow101
added a commit
that referenced
this pull request
Feb 17, 2023
…and txdb aadd7c5 refactor, validation: Add ChainstateManagerOpts db options (Ryan Ofsky) 0352258 refactor, txdb: Use DBParams struct in CBlockTreeDB (Ryan Ofsky) c00fa1a refactor, txdb: Add CoinsViewOptions struct (Ryan Ofsky) 2eaeded refactor, dbwrapper: Add DBParams and DBOptions structs (Ryan Ofsky) Pull request description: Code in the libbitcoin_kernel library should not be calling `ArgsManager` methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This PR removes `gArgs` accesses from `dbwrapper` and `txdb` modules by defining appropriate options structs, and is a followup to PR's #25290 #25487 #25527 which remove other `ArgsManager` calls from kernel modules. This PR does not change behavior in any way. It is a simpler alternative to #25623 because the only thing it does is remove `gArgs` references from kernel code. It avoids other unnecessary changes like adding options to the kernel API (they can be added separately later). ACKs for top commit: TheCharlatan: Code review ACK aadd7c5 achow101: ACK aadd7c5 furszy: diff ACK aadd7c5 Tree-SHA512: 46dfd5d99ab3110492e7bba97a87122c831b8344caaf7dd2ebdb6e0ad6aa9174d4d1832d6f3a7465eda9294fe50defaa3c000afbbddc4e72838687df09a63ffd
This was referenced Feb 19, 2023
fanquake
added a commit
to bitcoin-core/gui
that referenced
this pull request
Mar 14, 2023
…ystem.h aaced56 refactor: Move error() F438 from util/system.h to logging.h (Ben Woosley) e7333b4 refactor: Extract util/exception from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project bitcoin/bitcoin#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 (bitcoin/bitcoin#25290, bitcoin/bitcoin#25487, bitcoin/bitcoin#25527, bitcoin/bitcoin#25862, bitcoin/bitcoin#26177, and bitcoin/bitcoin#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 aaced56 🐍 Tree-SHA512: cb39f4cb7a77e7dc1887b1cbf340d53decab8880fc00878a2f12dc627fe67245b4aafd4cc31a9eab0fad1e5bb5d0eb4cdb8d501323ca200fa6ab7b201ae34aea
fanquake
added a commit
that referenced
this pull request
Mar 16, 2023
…lity to kernel b3e78dc refactor: Don't use global chainparams in chainstatemanager method (TheCharlatan) 382b692 Split non/kernel chainparams (Carl Dong) edabbc7 Add factory functions for Main/Test/Sig/Reg chainparams (Carl Dong) d938098 Remove UpdateVersionBitsParameters (Carl Dong) 84b8578 Decouple RegTestChainParams from ArgsManager (Carl Dong) 76cd4e7 Decouple SigNetChainParams from ArgsManager (Carl Dong) Pull request description: This pull request is part of the `libbitcoinkernel` project #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 #25290, #25487, #25527 and #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 b3e78dc 🛁 ryanofsky: Code review ACK b3e78dc. Only changes since last review were recent review suggestions. ajtowns: ACK b3e78dc Tree-SHA512: 3835aca1d3e3c75cc3303dd584bab3a77e58f6c678724a5e359fe4b0e17e0763a00931ee6191f516b9fde50496f59cc691f0709c0254206db3863bbf7ab2cacd
fanquake
added a commit
to bitcoin-core/gui
that referenced
this pull request
Apr 3, 2023
00e9b97 refactor: Move fs.* to util/fs.* (TheCharlatan) 106b46d Add missing fs.h includes (TheCharlatan) b202b3d Add missing cstddef include in assumptions.h (TheCharlatan) 18fb363 refactor: Extract util/fs_helpers from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project bitcoin/bitcoin#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 (bitcoin/bitcoin#25290, bitcoin/bitcoin#25487, bitcoin/bitcoin#25527, bitcoin/bitcoin#25862, bitcoin/bitcoin#26177, and bitcoin/bitcoin#27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in bitcoin/bitcoin#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 00e9b97 Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
fanquake
added a commit
to bitcoin-core/gui
that referenced
this pull request
May 11, 2023
…rom blockstorage 5ff63a0 refactor, blockstorage: Replace stopafterblockimport arg (TheCharlatan) 18e5ba7 refactor, blockstorage: Replace blocksdir arg (TheCharlatan) 02a0899 refactor, BlockManager: Replace fastprune from arg with options (TheCharlatan) a498d69 refactor/iwyu: Complete includes for blockmanager_args (TheCharlatan) f0bb102 refactor: Move functions to BlockManager methods (TheCharlatan) cfbb212 zmq: Pass lambda to zmq's ZMQPublishRawBlockNotifier (TheCharlatan) 8ed4ff8 refactor: Declare g_zmq_notification_interface as unique_ptr (TheCharlatan) Pull request description: The libbitcoin_kernel library should not rely on the `ArgsManager`, but rather use option structs that can be passed to the various classes it uses. This PR removes reliance on the `ArgsManager` from the `blockstorage.*` files. Like similar prior work, it uses the options struct in the `BlockManager` that can be populated with `ArgsManager` values. Some related prior work: bitcoin/bitcoin#26889 bitcoin/bitcoin#25862 bitcoin/bitcoin#25527 bitcoin/bitcoin#25487 Related PR removing blockstorage globals: bitcoin/bitcoin#25781 ACKs for top commit: ryanofsky: Code review ACK 5ff63a0. Since last ACK just added std::move and fixed commit title. Sorry for the noise! mzumsande: Code Review ACK 5ff63a0 Tree-SHA512: 4bde8fd140a40b97eca923e9016d85dcea6fad6fd199731f158376294af59c3e8b163a0725aa47b4be3519b61828044e0a042deea005e0c28de21d8b6c3e1ea7
ryanofsky
added a commit
that referenced
this pull request
Jun 9, 2023
…base from kernel library db77f87 scripted-diff: move settings to common namespace (TheCharlatan) c27e4bd move-only: Move settings to the common library (TheCharlatan) c2dae5d kernel: Remove chainparams, chainparamsbase, args, settings from kernel library (TheCharlatan) 05870b1 refactor: Remove gArgs access from validation.cpp (TheCharlatan) 8789b11 refactor: Add path argument to FindSnapshotChainstateDir (TheCharlatan) ef95be3 refactor: Add stop_at_height option in ChainstateManager (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project #27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". --- This completes the removal of the node's chainparams, chainparamsbase, args and settings files and their respective classes from the kernel library. This is the last pull request in a long series working towards decoupling the `ArgsManager` and the `gArgs` global from kernel code. These prior pull requests are: #26177 #27125 #25527 #25487 #25290 ACKs for top commit: MarcoFalke: lgtm ACK db77f87 🍄 hebasto: ACK db77f87, I have reviewed the code and it looks OK. ryanofsky: Code review ACK db77f87. Looks great! Tree-SHA512: cbfbd705d056f2f10f16810d4f869eb152362fff2c5ddae5e1ac6785deae095588e52ad48b29d921962b085e51de1e0ecab6e50f46149ffe3c16250608a2c93a
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This is part of the
libbitcoinkernelproject: #24303, https://github.com/bitcoin/bitcoin/projects/18This PR is NOT dependent on any other PRs.
a.k.a. "Stop calling
gArgs.GetIntArg("-maxsigcachesize")from validation code"This PR introduces the
ValidationCacheSizesstruct and its correspondingApplyArgsManOptionsfunction, removing the need to callgArgsfromInit{Signature,ScriptExecution}Cache(). This serves to further decoupleArgsManagerfromlibbitcoinkernelcode.More context can be gleaned from the commit messages.