aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
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-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-23refactor: Extract util/fs_helpers from util/systemBen Woosley
This is an extraction of filesystem related functions from util/system into their own utility file. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager defined in system.h. Moving these functions out of system.h allows including them from a separate source file without including the ArgsManager definitions from system.h.
2023-03-22Merge bitcoin/bitcoin#26749: refactor: Use move semantics instead of custom ↡fanquake
swap functions 95ad70ab652ddde7de65f633c36c1378b26a313a test: Default initialize `should_freeze` to `true` (Hennadii Stepanov) cea50521fe810111a8a3c84ad14f944eafb5b658 refactor: Drop no longer used `swap` member functions (Hennadii Stepanov) a87fb6bee5a7fb0879b3adea9a29997f1331acb0 clang-tidy: Fix modernize-use-default-member-init in `CScriptCheck` (Hennadii Stepanov) b4bed5c1f98c0eed18f52fdcea11a420c10ed98d refactor: Drop no longer used `CScriptCheck()` default constructor (Hennadii Stepanov) d8427cc28e3a9ac3319fb452b16661957c812b8f refactor: Use move semantics in `CCheckQueue::Loop` (Hennadii Stepanov) 9a0b5241396efe3b3ceb3931717c30bb94f99bfb clang-tidy, test: Fix bugprone-use-after-move in `Correct_Queue_range()` (Hennadii Stepanov) 04831fee6dca3eb86cd1d6b9ef879b296263fe35 refactor: Make move semantics explicit for callers (Hennadii Stepanov) 6c2d5972f3544c4f3e987828a99e88f27b62cf87 refactor: Use move semantics in `CCheckQueue::Add` (Hennadii Stepanov) 06820032142a75cc3c5b832045058bc6f6f74786 test, refactor: Avoid `CScriptCheck::swap` in `transaction_tests` (Hennadii Stepanov) 15209d97c6aad7d5c199fe007ad39b91c8ee6562 consensus, refactor: Avoid `CScriptCheck::swap` in `CheckInputScripts` (Hennadii Stepanov) Pull request description: This PR makes code more succinct and readable by using move semantics. ACKs for top commit: martinus: re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a achow101: ACK 95ad70ab652ddde7de65f633c36c1378b26a313a TheCharlatan: re-ACK https://github.com/bitcoin/bitcoin/commit/95ad70ab652ddde7de65f633c36c1378b26a313a MarcoFalke: re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a πŸš₯ Tree-SHA512: adda760891b12d252dc9b823fe7c41eed660364b6fb1a69f17607d7a31eb0bbb82a80d154a7acfaa241b5de37d42a293c2b6e059f26a8e92d88d3a87c99768fb
2023-03-21Merge bitcoin/bitcoin#27278: Log new headersAndrew Chow
2c3a90f663a61ee147d785167a2902494d81d34d log: on new valid header (James O'Beirne) e5ce8576349d404c466b2f4cab1ca7bf920904b2 log: net: new header over cmpctblock (James O'Beirne) Pull request description: Alternate to #27276. Devs were [suprised to realize](https://twitter.com/jamesob/status/1637237917201383425) last night that we don't have definitive logging for when a given header was first received. This logs to the main stream when new headers are received outside of IBD, as well as when headers come in over cmpctblocks. The rationale of not hiding these under log categories is that they may be useful to have widely available when debugging strange network activity, and the marginal volume is modest. ACKs for top commit: dergoegge: Code review ACK 2c3a90f663a61ee147d785167a2902494d81d34d achow101: ACK 2c3a90f663a61ee147d785167a2902494d81d34d Sjors: tACK 2c3a90f663a61ee147d785167a2902494d81d34d josibake: ACK https://github.com/bitcoin/bitcoin/pull/27278/commits/2c3a90f663a61ee147d785167a2902494d81d34d Tree-SHA512: 49fdcbe07799c8adc24143d7e5054a0c93fef120d2e9d5fddbd3b119550d895e2985be6ac10dd1825ea23a6fa5479c1b76d5518c136fbd983fa76c0d39dc354f
2023-03-21refactor: Make move semantics explicit for callersHennadii Stepanov
2023-03-21consensus, refactor: Avoid `CScriptCheck::swap` in `CheckInputScripts`Hennadii Stepanov
2023-03-20log: on new valid headerJames O'Beirne
2023-03-20Merge bitcoin/bitcoin#26531: mempool: Add mempool tracepointsAndrew Chow
4b7aec2951fe4595946cdc804b0dec1921d79d05 Add mempool tracepoints (virtu) Pull request description: This PR adds multiple mempool tracepoints. | tracepoint | description | | ------------- | ------------- | | `mempool:added` | Is called when a transaction enters the mempool | | `mempool:removed` | ... when a transaction is removed from the mempool | | `mempool:replaced` | ... when a transaction is replaced in the mempool | | `mempool:rejected` | ... when a transaction is rejected from entering the mempool | The tracepoints are further documented in `docs/tracing.md`. Usage is demonstrated in the example script `contrib/tracing/mempool_monitor.py`. Interface tests are provided in `test/functional/interface_usdt_mempool.py`. The rationale for passing the removal reason as a string instead of numerically is that the benefits of not having to maintain a redundant enum-string mapping seem to outweigh the small cost of string generation. The reject reason is passed as string as well, although in this instance the string does not have to be generated but is readily available. ACKs for top commit: 0xB10C: ACK 4b7aec2951fe4595946cdc804b0dec1921d79d05 achow101: ACK 4b7aec2951fe4595946cdc804b0dec1921d79d05 Tree-SHA512: 6deb3ba2d1a061292fb9b0f885f7a5c4d11b109b838102d8a8f4828cd68f5cd03fa3fc64adc6fdf54a08a1eaccce261b0aa90c2b8c33cd5fd3828c8f74978958
2023-03-20Add mempool tracepointsvirtu
Tracepoints for added, removed, replaced, and rejected transactions. The removal reason is passed as string instead of a numeric value, since the benefits of not having to maintain a redundant enum-string mapping seem to outweigh the small cost of string generation. The reject reason is passed as string as well, although here the string does not have to be generated but is readily available. So far, tracepoint PRs typically included two demo scripts: a naive bpftrace script to show raw tracepoint data and a bcc script for a more refined view. However, as some of the ongoing changes to bpftrace introduce a certain degree of unreliability (running some of the existing bpftrace scripts was not possible with standard kernels and bpftrace packages on latest stable Ubuntu, Debian, and NixOS), this PR includes only a single bcc script that fuses the functionality of former bpftrace and bcc scripts.
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-15refactor: Don't use global chainparams in chainstatemanager methodTheCharlatan
The chainstatemanager m_options.chainparams member variable gets its value from the global chainparams in init.cpp. This allows validation.cpp to only include the the kernel chainparams file.
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-07log: add LoadBlockIndex() message for assumedvalid blocksJames O'Beirne
I found this useful during unittest debugging.
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-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-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-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.
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: 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-01-31log: Log VerifyDB Progress over multiple linesMartin Zumsande
This allows to log a timestamp for each entry, and avoids potential interference with other threads that could log concurrently.
2023-01-31validation: Skip VerifyDB checks of level >=3 if dbcache is too smallMartin Zumsande
The previous behavior, skipping some L3 DisconnectBlock calls, but still attempting to reconnect these blocks at L4, makes ConnectBlock assert. The variable skipped_l3_checks is introduced because even with an insufficient cache for the L3 checks, the L1/L2 checks in the same loop should still be completed. Fixes #25563.
2023-01-31refactor: Move calculation logic out from `CheckSequenceLocksAtTip()`Hennadii Stepanov
2023-01-31refactor: Add `CalculateLockPointsAtTip()` functionHennadii Stepanov
2023-01-16Add BlockManager::IsPruneMode()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-11Merge bitcoin/bitcoin#26758: refactor: Add `performance-no-automatic-move` ↡MarcoFalke
clang-tidy check 9567bfeab95cc0932073641dd162903850987d43 clang-tidy: Add `performance-no-automatic-move` check (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1054673201). For the problem description see https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html. The following types are affected: - `std::pair<CAddress, NodeSeconds>` - `std::vector<CAddress>` - `UniValue`, also see bitcoin/bitcoin#25429 - `QColor` - `CBlock` - `MempoolAcceptResult` - `std::shared_ptr<CWallet>` - `std::optional<SelectionResult>` - `CTransactionRef`, which is `std::shared_ptr<const CTransaction>` ACKs for top commit: andrewtoth: ACK 9567bfeab95cc0932073641dd162903850987d43 aureleoules: ACK 9567bfeab95cc0932073641dd162903850987d43 Tree-SHA512: 9b6a5d539205b41d2c86402d384318ed2e1d89e66333ebd200a48fd7df3ce6f6c60a3e989eda5cc503fb34b8d82526f95e56776e1af51e63b49e3a1fef72dbcb
2023-01-10[validation] return MempoolAcceptResult for every tx on PCKG_TX failureglozow
This makes the interface more predictable and useful. The caller understands one or more transactions failed, and can learn what happened with each transaction. We already have this information, so we might as well return it. It doesn't make sense to do this for other PackageValidationResult values because: - PCKG_RESULT_UNSET: this means everything succeeded, so the individual failures are no longer accurate. - PCKG_MEMPOOL_ERROR: something went wrong with the mempool logic; transaction failures might not be meaningful. - PCKG_POLICY: this means something was wrong with the package as a whole. The caller should use the PackageValidationState to find the error, rather than looking at individual MempoolAcceptResults.
2023-01-10[refactor] rename variables in AcceptPackage for clarityglozow
2023-01-10[validation] remove PackageMempoolAcceptResult::m_package_feerateglozow
This value creates an extremely confusing interface as its existence is dependent upon implementation details (whether something was submitted on its own, etc). MempoolAcceptResult::m_effective_feerate is much more helpful, as it always exists for submitted transactions.
2023-01-10[validation] return wtxids of other transactions whose fees were usedglozow
2023-01-06[validation] return effective feerate from mempool validationglozow
2023-01-06[validation] when quitting early in AcceptPackage, set package_state and tx ↡glozow
result Bug: not setting package_state means package_state.IsValid() == true and the caller does not know that this failed. We won't be validating this transaction again, so it makes sense to return this failure to the caller. Rename package_state to package_state_quit_early to make it more clear what this variable is used for and what its scope is. Co-authored-by: Greg Sanders <gsanders87@gmail.com>
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
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
2022-12-27clang-tidy: Add `performance-no-automatic-move` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-21Merge bitcoin/bitcoin#26265: POLICY: Relax MIN_STANDARD_TX_NONWITNESS_SIZE ↡Andrew Chow
to 65 non-witness bytes b2aa9e85289fc654106a890c35935e9c76c411fb Add release note for MIN_STANDARD_TX_NONWITNESS_SIZE relaxation (Greg Sanders) 8c5b3646b5afe8a61f5c66478d8e11f0d2ce5108 Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes (Greg Sanders) Pull request description: Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was proposed as a consensus change, and is the simpler of the two suggestions. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes Related mailing list discussions here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/020995.html And a couple years earlier: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-May/017883.html ACKs for top commit: achow101: reACK b2aa9e85289fc654106a890c35935e9c76c411fb glozow: reACK b2aa9e85289fc654106a890c35935e9c76c411fb pablomartin4btc: re-ACK https://github.com/bitcoin/bitcoin/commit/b2aa9e85289fc654106a890c35935e9c76c411fb jonatack: ACK b2aa9e85289fc654106a890c35935e9c76c411fb with some suggestions Tree-SHA512: c1ec1af9ddcf31b2272209a4f1ee0c5607399f8172e5a1dfd4604cf98bfb933810dd9369a5917ad122add003327c9fcf6ee26995de3aca41d5c42dba527991ad
2022-12-20prune: scan and unlink already pruned block files on startupAndrew Toth
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-21 15:28:05 +0000