aboutsummaryrefslogtreecommitdiff
path: root/src/validation.h
AgeCommit message (Collapse)Author
2023-09-13Merge bitcoin/bitcoin#28251: validation: fix coins disappearing mid-package ↵fanquake
evaluation 32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 [test] mempool coins disappearing mid-package evaluation (glozow) a67f460c3fd1c7eb8070623666d887eefccff0d6 [refactor] split setup in mempool_limit test (glozow) d08696120e3647b4c2cd0ae8d6e57dea12418b7c [test framework] add ability to spend only confirmed utxos (glozow) 3ea71feb11c261f002ed918f91f3434fd8a23589 [validation] don't LimitMempoolSize in any subpackage submissions (glozow) d227b7234cd4cfd7c593ffcf8e2f24573d1ebea5 [validation] return correct result when already-in-mempool tx gets evicted (glozow) 9698b81828ff98820fa49c83ca364063233374c6 [refactor] back-fill results in AcceptPackage (glozow) 8ad7ad33929ee846a55a43c55732be0cb8973060 [validation] make PackageMempoolAcceptResult members mutable (glozow) 03b87c11ca0705e1d6147b90da33ce555f9f41c8 [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow) 3f01a3dab1c4ee37fd4093b6a0a3b622f53e231d [CCoinsViewMemPool] track non-base coins and allow Reset (glozow) 7d7f7a1189432b1b6245ba25df572229870567cb [policy] check for duplicate txids in package (glozow) Pull request description: While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore. Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out. Pointed out by instagibbs in https://github.com/bitcoin/bitcoin/commit/faeed687e5cde5e32750d93818dd1d4add837f24 on top of the v3 PR. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/28251/commits/32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90
2023-09-13[validation] make PackageMempoolAcceptResult members mutableglozow
After the PackageMempoolAcceptResult is returned from AcceptMultipleTransactions, leave room for results to change due to LimitMempool() eviction.
2023-09-05Merge bitcoin/bitcoin#28195: blockstorage: Drop legacy -txindex checkfanquake
fae405556d56f6f13ce57f69a06b9ec1e825422b scripted-diff: Rename CBlockTreeDB -> BlockTreeDB (MarcoFalke) faf63039cce40f5cf8dea5a1d24945773c3433a1 Fixup style of moved code (MarcoFalke) fa65111b99627289fd47dcfaa5197e0f09b8a50e move-only: Move CBlockTreeDB to node/blockstorage (MarcoFalke) fa8685597e7302fc136f21b6dd3a4b187fa8e251 index: Drop legacy -txindex check (MarcoFalke) fa69148a0a26c5054dbccdceeac8e117bf449275 scripted-diff: Use blocks_path where possible (MarcoFalke) Pull request description: The only reason for the check was to print a warning about an increase in storage use. Now that 22.x is EOL and everyone should have migrated (or decided to not care about storage use), remove the check. Also, a move-only commit is included. (Rebased from https://github.com/bitcoin/bitcoin/pull/22242) ACKs for top commit: TheCharlatan: ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251. stickies-v: ACK fae405556d56f6f13ce57f69a06b9ec1e825422b Tree-SHA512: 9da8f48767ae52d8e8e21c09a40c949cc0838794f1856cc5f58a91acd3f00a3bca818c8082242b3fdc9ca5badb09059570bb3870850d3807b75a8e23b5222da1
2023-08-18assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ↵Ryan Ofsky
ChainstateManager This change makes IsInitialBlockDownload and NotifyHeaderTip functions no longer tied to individual Chainstate objects. It makes them work with the ChainstateManager object instead so code is simpler and it is no longer possible to call them incorrectly with an inactive Chainstate. This change also makes m_cached_finished_ibd caching easier to reason about, because now there is only one cached value instead of two (for background and snapshot chainstates) so the cached IBD state now no longer gets reset when a snapshot is loaded. There should be no change in behavior because these functions were always called on the active ChainState objects. These changes were discussed previously https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1246868905 and https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1237552792 as possible followups for that PR.
2023-08-17Merge bitcoin/bitcoin#27675: p2p: Drop m_recently_announced_invs bloom filterfanquake
fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 mempool_entry: improve struct packing (Anthony Towns) 1a118062fbc4ec8f645f4ec4298d869a869c3344 net_processing: Clean up INVENTORY_BROADCAST_MAX constants (Anthony Towns) 6fa49937e488d0924044786c76b42324b659f351 test: Check tx from disconnected block is immediately requestable (glozow) e4ffabbffacc4b890d393aafcc8286916ef887d8 net_processing: don't add txids to m_tx_inventory_known_filter (Anthony Towns) 6ec1809d33bfc42b80cb6f35625dccd56be8d507 net_processing: drop m_recently_announced_invs bloom filter (Anthony Towns) a70beafdb22564043dc24fc98133fdadbaf77d8a validation: when adding txs due to a block reorg, allow immediate relay (Anthony Towns) 1e9684f39fba909b3501e9402d5b61f4bf744ff2 mempool_entry: add mempool entry sequence number (Anthony Towns) Pull request description: This PR replaces the `m_recently_announced_invs` bloom filter with a simple sequence number tracking the mempool state when we last considered sending an INV message to a node. This saves 33kB per peer (or more if we raise the rate at which we relay transactions over the network, in which case we would need to increase the size of the bloom filter proportionally). The philosophy here (compare with #18861 and #19109) is that we consider the rate limiting on INV messages to only be about saving bandwidth and not protecting privacy, and therefore after you receive an INV message, it's immediately fair game to request any transaction that was in the mempool at the time the INV message was sent. We likewise consider the BIP 133 feefilter and BIP 37 bloom filters to be bandwidth optimisations here, and treat transactions as requestable if they would have been announced without those filters. Given that philosophy, tracking the timestamp of the last INV message and comparing that against the mempool entry time allows removal of each of `m_recently_announced_invs`, `m_last_mempool_req` and `UNCONDITIONAL_RELAY_DELAY` and associated logic. ACKs for top commit: naumenkogs: ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 amitiuttarwar: review ACK fb02ba3c5f5 glozow: reACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 Tree-SHA512: cbba5ee04c86df26b6057f3654c00a2b45ec94d354f4f157a769cecdaa0b509edaac02b3128afba39b023e82473fc5e28c915a787f84457ffe66638c6ac9c2d4
2023-08-07Remove Chainstate::LoadMempoolMarcoFalke
The 3-line function is only called once outside of tests, so it is clearer to inline it.
2023-08-03validation: when adding txs due to a block reorg, allow immediate relayAnthony Towns
2023-08-01move-only: Move CBlockTreeDB to node/blockstorageMarcoFalke
The block index (CBlockTreeDB) is required to write and read blocks, so move it to blockstorage. This allows to drop the txdb.h include from `node/blockstorage.h`. Can be reviewed with: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2023-07-24Remove unused function `reliesOnAssumedValid`Suhas Daftuar
2023-07-24Cache block index entry corresponding to assumeutxo snapshot base blockhashSuhas Daftuar
This is to (a) avoid repeated lookups into the block index for an entry that should never change and (b) emphasize that the snapshot base should always exist when set and not change during the runtime of the program. Thanks to Russ Yanofsky for suggesting this approach.
2023-07-24Move CheckBlockIndex() from Chainstate to ChainstateManagerSuhas Daftuar
Also rewrite CheckBlockIndex() to perform tests on all chainstates. This increases sanity-check coverage, as any place in our code where we were invoke CheckBlockIndex() on a single chainstate will now invoke the sanity checks on all chainstates. This change also tightens up the checks on setBlockIndexCandidates and mapBlocksUnlinked, to more precisely match what we aim for even in the presence of assumed-valid blocks.
2023-07-21Move block-storage-related logic to ChainstateManagerSuhas Daftuar
Separate the notion of which blocks are stored on disk, and what data is in our block index, from what tip a chainstate might be able to get to. We can use chainstate-agnostic data to determine when to store a block on disk (primarily, an anti-DoS set of criteria) and let the chainstates figure out for themselves when a block is of interest for being a candidate tip. Note: some of the invariants in CheckBlockIndex are modified, but more work is needed (ie to move CheckBlockIndex to ChainstateManager, as most of what CheckBlockIndex is doing is checking the consistency of the block index, which is outside of Chainstate).
2023-07-14Add wrapper for adding entries to a chainstate's block index candidatesSuhas Daftuar
2023-07-14Move block-arrival information / preciousblock counters to ChainstateManagerSuhas Daftuar
Block arrival information (and the preciousblock RPC, a related concept) are both chainstate-agnostic, so these are moved to ChainstateManager. This should just be a refactor, without any observable behavior changes.
2023-07-11kernel: Remove StartShutdown calls from validation codeRyan Ofsky
This change drops the last kernel dependency on shutdown.cpp. It also adds new hooks for libbitcoinkernel applications to be able to interrupt kernel operations when the chain tip changes. This is a refactoring that does not affect behavior. (Looking at the code it can appear like the new break statement in the ActivateBestChain function is a change in behavior, but actually the previous StartShutdown call was indirectly triggering a break before, because it was causing m_chainman.m_interrupt to be true. The new code just makes the break more obvious.)
2023-07-07scripted-diff: rename 'loadblk' thread name to 'initload'furszy
The thread does not only load blocks, it loads the mempool and, in a future commit, will start the indexes as well. Also, renamed the 'ThreadImport' function to 'ImportBlocks' And the 'm_load_block' class member to 'm_thread_load'. -BEGIN VERIFY SCRIPT- sed -i "s/ThreadImport/ImportBlocks/g" $(git grep -l ThreadImport -- ':!/doc/') sed -i "s/loadblk/initload/g" $(git grep -l loadblk -- ':!/doc/release-notes/') sed -i "s/m_load_block/m_thread_load/g" $(git grep -l m_load_block) -END VERIFY SCRIPT-
2023-06-28kernel: Add fatalError method to notificationsTheCharlatan
FatalError replaces what previously was the AbortNode function in shutdown.cpp. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-28kernel: Pass interrupt reference to chainmanTheCharlatan
This and the following commit seek to decouple the libbitcoinkernel library from the shutdown code. As a library, it should it should have its own flexible interrupt infrastructure without relying on node-wide globals. The commit takes the first step towards this goal by de-globalising `ShutdownRequested` calls in kernel code. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-27Merge bitcoin/bitcoin#27334: util: implement `noexcept` move assignment & ↵Andrew Chow
move ctor for `prevector` bfb9291a8661fe5b26c14ed755cfa89d27c37110 util: implement prevector's move ctor & move assignment (Martin Leitner-Ankerl) fffc86f49f4eeb811b8438bc1b7f8d9e05882c6f test: CScriptCheck is used a lot in std::vector, make sure that's efficient (Martin Leitner-Ankerl) 81f67977f543faca2dcc35846f73e2917375ae79 util: prevector's move ctor and move assignment is `noexcept` (Martin Leitner-Ankerl) d380d2877ed45cf1e75a87d822b30e4e1e21e3d4 bench: Add benchmark for prevector usage in std::vector (Martin Leitner-Ankerl) Pull request description: `prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector. The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it. merge-base: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,440.29 | 155,272.42 | 0.2% | 40,713.01 | 20,473.84 | 1.989 | 7,132.01 | 0.2% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 3,213.19 | 311,217.35 | 0.7% | 35,373.01 | 10,214.07 | 3.463 | 6,945.00 | 0.2% | 0.43 | `PrevectorFillVectorDirectTrivial` | 34,749.70 | 28,777.23 | 0.1% | 364,396.05 | 110,521.94 | 3.297 | 78,568.37 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 32,535.05 | 30,736.09 | 0.4% | 353,823.31 | 103,464.53 | 3.420 | 79,871.80 | 0.2% | 0.40 | `PrevectorFillVectorIndirectTrivial` util: prevector's move ctor and move assignment is `noexcept`: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,603.87 | 151,426.40 | 0.2% | 23,734.01 | 21,009.63 | 1.130 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,980.93 | 504,813.15 | 0.1% | 13,784.00 | 6,304.32 | 2.186 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 19,110.54 | 52,327.15 | 0.1% | 139,816.41 | 51,987.72 | 2.689 | 28,512.18 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 12,334.37 | 81,074.27 | 0.7% | 125,655.12 | 39,253.58 | 3.201 | 27,854.46 | 0.2% | 0.44 | `PrevectorFillVectorIndirectTrivial` util: implement prevector's move ctor & move assignment | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,262.66 | 190,018.01 | 0.2% | 20,157.01 | 16,745.26 | 1.204 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,687.07 | 592,744.35 | 0.2% | 12,742.00 | 5,368.02 | 2.374 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 17,930.80 | 55,769.95 | 0.1% | 136,237.69 | 47,903.31 | 2.844 | 28,512.02 | 0.2% | 0.42 | `PrevectorFillVectorIndirectNontrivial` | 11,893.75 | 84,077.78 | 0.2% | 126,182.02 | 37,852.91 | 3.333 | 28,152.01 | 0.1% | 0.44 | `PrevectorFillVectorIndirectTrivial` As can be seen, mostly thanks to just `noexcept` the benchmark becomes about 2 times faster because `std::vector` can now use move operations instead of having to fall back to copy everything I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is `CCheckQueueSpeedPrevectorJob` goes from 364.56ns down to 346.21ns. ACKs for top commit: achow101: ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 jonatack: > Tested Approach ACK [bfb9291](https://github.com/bitcoin/bitcoin/commit/bfb9291a8661fe5b26c14ed755cfa89d27c37110), ~ACK modulo re-verifying the implementation of the last commit. john-moffett: Approach ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 theStack: Tested and light code-review ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 Tree-SHA512: 242995d7cb2f8ebfa73177aa690a505f189d91edeb8cea3e34a41ca507c8cb17c65fe2a4e196fdafc5c6e89b2b2222627bfc9f5c316517de0857b7b5e9c58225
2023-06-15validation: Stricter assumeutxo error handling in InvalidateCoinsDBOnDiskRyan Ofsky
Currently InvalidateCoinsDBOnDisk is calling AbortNode without an error to the caller if it fails. Change it to return just return util::Result, and update the caller to handle the error itself. This causes the secondary error to be shown below the main error instead of the other way around.
2023-06-12Merge bitcoin/bitcoin#27357: validation: Move warningcache to ↵fanquake
ChainstateManager and rename to m_warningcache 552684976b6df34ce563458f73812e6e494e3b0e validation: Move warningcache to ChainstateManager (dimitaracev) Pull request description: Removes `warningcache` and moves it to `ChainstateManager`. Also removes the respective `TODO` completely. ACKs for top commit: ajtowns: ACK 552684976b6df34ce563458f73812e6e494e3b0e dimitaracev: > ACK [5526849](https://github.com/bitcoin/bitcoin/commit/552684976b6df34ce563458f73812e6e494e3b0e) TheCharlatan: ACK 552684976b6df34ce563458f73812e6e494e3b0e ryanofsky: Code review ACK 552684976b6df34ce563458f73812e6e494e3b0e Tree-SHA512: 6869bd7aa4f0b59324e12eb8e3df47f2c9a3f3b0d9b7d45857426ec9e8b71c5573bdcf71db822f8c10aff7d8679a00a4bedc7a256c28f325e744e5d7267b41e9
2023-05-30refactor: Add stop_at_height option in ChainstateManagerTheCharlatan
Remove access to the global gArgs for the stopatheight argument and replace it by adding a field to the existing ChainstateManager Options struct. This should eventually allow users of the ChainstateManager to not rely on the global gArgs and instead pass in their own options.
2023-05-20kernel: Add progress method to notificationsTheCharlatan
This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index.
2023-05-20kernel: Add notification interfaceTheCharlatan
This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. Define a new kernel notification class with virtual methods for notifying about internal kernel events. Create a new file in the node library for defining a function creating the default set of notification methods such that these do not need to be re-defined all over the codebase. As a first step, add a `blockTip` method, wrapping `uiInterface.NotifyBlockTip`.
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-30[validation] Annotate ChainstateManager::m_best_header as guarded by cs_maindergoegge
2023-03-29validation: Move warningcache to ChainstateManagerdimitaracev
2023-03-26test: CScriptCheck is used a lot in std::vector, make sure that's efficientMartin Leitner-Ankerl
Adds a few static_asserts so CScriptCheck stays is_nothrow_move_assignable, is_nothrow_move_constructible, and is_nothrow_destructible
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-21refactor: Drop no longer used `swap` member functionsHennadii Stepanov
2023-03-21clang-tidy: Fix modernize-use-default-member-init in `CScriptCheck`Hennadii Stepanov
2023-03-21refactor: Drop no longer used `CScriptCheck()` default constructorHennadii Stepanov
2023-03-21consensus, refactor: Avoid `CScriptCheck::swap` in `CheckInputScripts`Hennadii Stepanov
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-15Move ::nPruneTarget into BlockManagerMarcoFalke
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-07refactor: make MempoolMutex() publicJames O'Beirne
for use in the following unittests.
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 FlushStateToDiskMarcoFalke
2023-02-28Merge bitcoin/bitcoin#23897: refactor: Move calculation logic out from ↵glozow
`CheckSequenceLocksAtTip()` 75db62ba4cae048e742ca02dc6a52b3b3d6727de refactor: Move calculation logic out from `CheckSequenceLocksAtTip()` (Hennadii Stepanov) 3bc434f4590758db673e1bd4ebf1906ea632f593 refactor: Add `CalculateLockPointsAtTip()` function (Hennadii Stepanov) Pull request description: This PR is follow up for bitcoin/bitcoin#22677 and bitcoin/bitcoin#23683. On master (013daed9acca1b723f599d63ab36b9c2a5c60e5f) it is not obvious that `CheckSequenceLocksAtTip()` function can modify its `LockPoints* lp` parameter which leads to https://github.com/bitcoin/bitcoin/pull/22677#discussion_r762040101. This PR: - separates the lockpoint calculate logic from `CheckSequenceLocksAtTip()` function into a new `CalculateLockPointsAtTip()` one - cleans up the `CheckSequenceLocksAtTip()` function interface - makes code easier to reason about (hopefully) ACKs for top commit: achow101: ACK 75db62ba4cae048e742ca02dc6a52b3b3d6727de stickies-v: re-ACK 75db62b Tree-SHA512: 072c3fd9cd1e1b0e0bfc8960a67b01c80a9f16d6778f374b6944ade03a020415ce8b8ab2593b0f5e787059c8cf90af798290b4c826785d41955092f6e12e7486
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-22add Chainstate::HasCoinsViews()James O'Beirne
Used in subsequent commits. Also cleans up asserts in coins_views-related convenience methods to be more exact.
2023-02-22validation: add CChainState::m_disabled and ChainMan::isUsableJames O'Beirne
and remove m_snapshot_validated. This state can now be inferred by the number of isUsable chainstates. m_disabled is used to signal that a chainstate should no longer be used by validation logic; it is used as a sentinel when background validation completes or if the snapshot chainstate is found to be invalid. isUsable is a convenience method that incorporates m_disabled.
2023-02-22add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}()James O'Beirne
For use in later commits.
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.