aboutsummaryrefslogtreecommitdiff
path: root/src/validation.h
AgeCommit message (Collapse)Author
2022-10-19Merge bitcoin/bitcoin#25830: refactor: Replace m_params with ↵MacroFake
chainman.GetParams() 5d3f98d27879cd6d84b8590e947336e8d09613ed refactor: Replace m_params with chainman.GetParams() (Aurèle Oulès) Pull request description: Fixes a TODO introduced in #24595. Removes `m_params` from `CChainState` class and replaces it with `m_chainman.GetParams()`. ACKs for top commit: MarcoFalke: review ACK 5d3f98d27879cd6d84b8590e947336e8d09613ed 🌎 Tree-SHA512: de0fe31450d281cc7307c0d820495e86c93c7998e77a148db2c703da66cff1059e6560c041f1864913c42075aa24d259c2623d45e929ca0a8056ed330a9f9978
2022-10-13Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initializationAndrew Chow
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne) e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne) cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne) 51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne) 3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne) 00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne) 3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne) 8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne) ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne) 34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne) 252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne) f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne) d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (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) --- Half of the replacement for #24232. The original PR grew larger than expected throughout the review process. This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots. Don't be scared! There are some big move-only commits in here. Accompanying changes include: - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880). - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init. - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization. ACKs for top commit: naumenkogs: utACK bf9597606166323158bbf631137b82d41f39334f ariard: Code Review ACK bf9597606 ryanofsky: Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests fjahr: utACK bf9597606166323158bbf631137b82d41f39334f aureleoules: ACK bf9597606166323158bbf631137b82d41f39334f Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-10-10refactor: Replace m_params with chainman.GetParams()Aurèle Oulès
Fixes a TODO introduced in #24595.
2022-10-03refactor: move DEFAULT_BLOCKFILTERINDEX from val to blockfilterindexfanquake
2022-10-03refactor: move DEFAULT_COINSTATSINDEX from validation to coinstatsindexfanquake
2022-10-03refactor: move DEFAULT_TXINDEX from validation to txindexfanquake
2022-09-13validation: add ResetChainstates()James O'Beirne
Necessary for the following test commit.
2022-09-13init: add utxo snapshot detectionJames O'Beirne
Add functionality for activating a snapshot-based chainstate if one is detected on-disk. Also cautiously initialize chainstate cache usages so that we don't somehow blow past our cache allowances during initialization, then rebalance at the end of init. Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2022-09-09scripted-diff: rename CChainState -> ChainstateJames O'Beirne
-BEGIN VERIFY SCRIPT- sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*') -END VERIFY SCRIPT- Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-08-30Merge bitcoin/bitcoin#25717: p2p: Implement anti-DoS headers syncfanquake
3add23454624c4c79c9eebc060b6fbed4e3131a7 ui: show header pre-synchronization progress (Pieter Wuille) 738421c50f2dbd7395b50a5dbdf6168b07435e62 Emit NotifyHeaderTip signals for pre-synchronization progress (Pieter Wuille) 376086fc5a187f5b2ab3a0d1202ed4e6c22bdb50 Make validation interface capable of signalling header presync (Pieter Wuille) 93eae27031a65b4156df49015ae45b2b541b4e5a Test large reorgs with headerssync logic (Suhas Daftuar) 355547334f7d08640ee1fa291227356d61145d1a Track headers presync progress and log it (Pieter Wuille) 03712dddfbb9fe0dc7a2ead53c65106189f5c803 Expose HeadersSyncState::m_current_height in getpeerinfo() (Suhas Daftuar) 150a5486db50ff77c91765392149000029c8a309 Test headers sync using minchainwork threshold (Suhas Daftuar) 0b6aa826b53470c9cc8ef4a153fa710dce80882f Add unit test for HeadersSyncState (Suhas Daftuar) 83c6a0c5249c4ecbd11f7828c84a50fb473faba3 Reduce spurious messages during headers sync (Suhas Daftuar) ed6cddd98e32263fc116a4380af6d66da20da990 Require callers of AcceptBlockHeader() to perform anti-dos checks (Suhas Daftuar) 551a8d957c4c44afbd0d608fcdf7c6a4352babce Utilize anti-DoS headers download strategy (Suhas Daftuar) ed470940cddbeb40425960d51cefeec4948febe4 Add functions to construct locators without CChain (Pieter Wuille) 84852bb6bb3579e475ce78fe729fd125ddbc715f Add bitdeque, an std::deque<bool> analogue that does bit packing. (Pieter Wuille) 1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51 Add function to validate difficulty changes (Suhas Daftuar) Pull request description: New nodes starting up for the first time lack protection against DoS from low-difficulty headers. While checkpoints serve as our protection against headers that fork from the main chain below the known checkpointed values, this protection only applies to nodes that have been able to download the honest chain to the checkpointed heights. We can protect all nodes from DoS from low-difficulty headers by adopting a different strategy: before we commit to storing a header in permanent storage, first verify that the header is part of a chain that has sufficiently high work (either `nMinimumChainWork`, or something comparable to our tip). This means that we will download headers from a given peer twice: once to verify the work on the chain, and a second time when permanently storing the headers. The p2p protocol doesn't provide an easy way for us to ensure that we receive the same headers during the second download of peer's headers chain. To ensure that a peer doesn't (say) give us the main chain in phase 1 to trick us into permanently storing an alternate, low-work chain in phase 2, we store commitments to the headers during our first download, which we validate in the second download. Some parameters must be chosen for commitment size/frequency in phase 1, and validation of commitments in phase 2. In this PR, those parameters are chosen to both (a) minimize the per-peer memory usage that an attacker could utilize, and (b) bound the expected amount of permanent memory that an attacker could get us to use to be well-below the memory growth that we'd get from the honest chain (where we expect 1 new block header every 10 minutes). After this PR, we should be able to remove checkpoints from our code, which is a nice philosophical change for us to make as well, as there has been confusion over the years about the role checkpoints play in Bitcoin's consensus algorithm. Thanks to Pieter Wuille for collaborating on this design. ACKs for top commit: Sjors: re-tACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 mzumsande: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 sipa: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 glozow: ACK 3add234546 Tree-SHA512: e7789d65f62f72141b8899eb4a2fb3d0621278394d2d7adaa004675250118f89a4e4cb42777fe56649d744ec445ad95141e10f6def65f0a58b7b35b2e654a875
2022-08-29Track headers presync progress and log itPieter Wuille
2022-08-29Require callers of AcceptBlockHeader() to perform anti-dos checksSuhas Daftuar
In order to prevent memory DoS, we must ensure that we don't accept a new header into memory until we've performed anti-DoS checks, such as verifying that the header is part of a sufficiently high work chain. This commit adds a new argument to AcceptBlockHeader() so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation. This patch also fixes two places where low-difficulty-headers could have been processed without such validation (processing an unrequested block from the network, and processing a compact block). Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost for test code.
2022-08-29Utilize anti-DoS headers download strategySuhas Daftuar
Avoid permanently storing headers from a peer, unless the headers are part of a chain with sufficiently high work. This prevents memory attacks using low-work headers. Designed and co-authored with Pieter Wuille.
2022-08-22refactor: Move ChainstateManager options into m_options structRyan Ofsky
Move ChainstateManager options into m_options struct to simplify class initialization, organize class members, and to name external option variables differently than internal state variables. This change was originally in #25862, but it was suggested to split off in https://github.com/bitcoin/bitcoin/pull/25862#discussion_r951459817 so it could be merged earlier and reduce conflicts with other PRs.
2022-08-22Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125fanquake
1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e [doc] remove non-signaling mentions of BIP125 (glozow) 32024d40f03fbf47c64d814fa5f2c2a73ec14cb7 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow) Pull request description: We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy. We should not use "BIP125" as synonymous with our RBF policy because: - Our RBF policy is different from what is specified in BIP125, for example: - the BIP does not mention our rule about the replacement feerate being higher (our Rule 6) - the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380 - the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md - the signaling policy is configurable, see #25353 - Our RBF policy may change further - We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498c75a1d14193bb736d195a3dc75ae12 - See comments from people who are not me recently: - https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429 - https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204 This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because: - It is succint. - It has already been widely marketed as BIP125 opt-in signaling. - Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive. - If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from. Alternatives: - Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6). - Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places. - Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step. ACKs for top commit: darosior: ACK 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e ariard: ACK 1dc03dda t-bast: ACK https://github.com/bitcoin/bitcoin/commit/1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02
2022-08-22Merge bitcoin/bitcoin#25786: refactor: Make adjusted time type safefanquake
eeee5ada23f2a71d245671556b6ecfdaabfeddf4 Make adjusted time type safe (MacroFake) fa3be799fe951a7ea9b4de78d5a907c6db71eeb8 Add time helpers (MacroFake) Pull request description: This makes follow-ups easier to review. Also, it makes sense by itself. ACKs for top commit: ryanofsky: Code review ACK eeee5ada23f2a71d245671556b6ecfdaabfeddf4. Confirmed type changes and equivalent code changes only. Tree-SHA512: 51bf1ae5428552177286113babdd49e82459d6c710a07b6e80a0a045d373cf51045ee010461aba98e0151d8d71b9b3b5f8f73e302d46ba4558e0b55201f99e9f
2022-08-16refactor: Add lock annotations to Active* methodsMacroFake
This is a refactor, putting the burden to think about thread safety to the caller. Otherwise, there is a risk that the caller will assume thread safety where none exists, as is evident in the previous two commits.
2022-08-16Add ChainstateManager::GetMutex(), an alias for ::cs_mainMacroFake
2022-08-12Merge bitcoin/bitcoin#25677: refactor: make active_chain_tip a referenceMacroFake
9376a6dae41022874df3b9302667796a9fb7b81d refactor: make active_chain_tip a reference (Aurèle Oulès) Pull request description: This PR fixes a TODO introduced in #21055. Makes `active_chain_tip` argument in `CheckFinalTxAtTip` function a reference instead of a pointer. ACKs for top commit: dongcarl: ACK 9376a6dae41022874df3b9302667796a9fb7b81d Tree-SHA512: c36d1769e0b9598b7f79334704b26b73e958d54caa3bd7e4eff954f3964fcf3f5e3a44a5a760497afad51b76e1614c86314fe035e4083c855e3574a620de7f4d
2022-08-09Merge bitcoin/bitcoin#24564: doc: Clarify that CheckSequenceLocksAtTip is a ↵glozow
validation function fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke) Pull request description: It has been pointed out that a bug in this function can prevent block template creation. ( https://github.com/bitcoin/bitcoin/pull/24080#issuecomment-1065148776 ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35. ACKs for top commit: ajtowns: ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d - looks fine to me glozow: ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
2022-08-05Make adjusted time type safeMacroFake
2022-08-04[doc] remove non-signaling mentions of BIP125glozow
Our RBF policy is different from the rules specified in BIP125. For example, the BIP does not mention Rule 6, and our Rule 4 uses the (configurable) incremental relay feerate (distinct from the minimum relay feerate). Those interested in our policy should refer to doc/policy/mempool-replacements.md instead. These rules may also continue to diverge with package RBF and other RBF improvements. Keep references to the BIP125 signaling wrt sequence numbers, since that is still correct and widely used. It is helpful to refer to this as "BIP125 signaling" since it is unambiguous and succint, especially if we have multiple ways to signal replaceability in the future. The rule numbers in doc/policy/mempool-replacements.md correspond largely to those of BIP 125, so we can still refer to them like "Rule 5."
2022-08-03validationcaches: Use size_t for sizesCarl Dong
...also move the 0-clamping logic to ApplyArgsManOptions, where it belongs.
2022-08-03validationcaches: Add and use ValidationCacheSizesCarl Dong
Also: - Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer arithmetic overflow checking available to constexpr. - Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES. - Pass in max_size_bytes parameter to InitS*Cache(), modify log line to no longer allude to maxsigcachesize being split evenly between the two validation caches. - Fix possible integer truncation and add a comment. [META] I've kept the integer types as int64_t in order to not introduce unintended behaviour changes, in the next commit we will make them size_t.
2022-08-03cuckoocache: Return approximate memory sizeCarl Dong
Returning the approximate total size eliminates the need for InitS*Cache() to do nElems*sizeof(uint256). The cuckoocache has a better idea of this information.
2022-08-02Remove ::fRequireStandard globalMacroFake
2022-07-29Merge bitcoin/bitcoin#25571: refactor: Make mapBlocksUnknownParent local, ↵fanquake
and rename it dd065dae9fcebd6806ff67703ffa8128e80b97cc refactor: Make mapBlocksUnknownParent local, and rename it (Hennadii Stepanov) Pull request description: This PR is a second attempt at #19594. This PR has two motivations: - Improve code hygiene by eliminating a global variable, `mapBlocksUnknownParent` - Fix fuzz test OOM when running too long ([see #19594 comment](https://github.com/bitcoin/bitcoin/pull/19594#issuecomment-958801638)) A minor added advantage is to release `mapBlocksUnknownParent` memory when the reindexing phase is done. The current situation is somewhat similar to a memory leak because this map exists unused for the remaining lifetime of the process. It's true that this map should be empty of data elements after use, but its internal metadata (indexing structures, etc.) can have non-trivial size because there can be many thousands of simultaneous elements in this map. This PR helps our efforts to reduce the use of global variables. This variable isn't just global, it's hidden inside a function (it looks like a local variable but has the `static` attribute). This global variable exists because the `-reindex` processing code calls `LoadExternalBlockFile()` multiple times (once for each block file), but that function must preserve some state between calls (the `mapBlocksUnknownParent` map). This PR fixes this by allocating this map as a local variable in the caller's scope and passing it in on each call. When reindexing completes, the map goes out of scope and is deallocated. I tested this manually by reindexing on mainnet and signet. Also, the existing `feature_reindex.py` functional test passes. ACKs for top commit: mzumsande: re-ACK dd065dae9fcebd6806ff67703ffa8128e80b97cc theStack: re-ACK dd065dae9fcebd6806ff67703ffa8128e80b97cc shaavan: reACK dd065dae9fcebd6806ff67703ffa8128e80b97cc Tree-SHA512: 9cd20e44d2fa1096dd405bc107bc065ea8f904f5b3f63080341b08d8cf57b790df565f58815c2f331377d044d5306708b4bf6bdfc5ef8d0ed85d8e97d744732c
2022-07-22refactor: make active_chain_tip a referenceAurèle Oulès
2022-07-20Merge bitcoin/bitcoin#25285: Add AutoFile without ser-type and ser-version ↵fanquake
and use it where possible facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Use AutoFile where possible (MacroFake) 6666803c897e4ad27b45cb74e3a9aa74a335f1bf streams: Add AutoFile without ser-type and ser-version (MacroFake) 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`. `CAutoFile` remains in places where it is not yet possible. ACKs for top commit: laanwj: Code review ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3 fanquake: ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Tree-SHA512: d82d024d55af57565ac53d9d1517afafc12b46964effba0332de62a6c77869356fa77f89e6d4834438fff44c45b64fccdf5a1358bfea03e28dfe55013b3c099d
2022-07-18refactor: Make mapBlocksUnknownParent local, and rename itHennadii Stepanov
Co-authored-by: Larry Ruane <larryruane@gmail.com>
2022-07-18Merge bitcoin/bitcoin#25487: [kernel 3b/n] Decouple `{Dump,Load}Mempool` ↵glozow
from `ArgsManager` cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Move {Load,Dump}Mempool to kernel namespace (Carl Dong) aa306765419f7dbea12b12e15553039835ba0e4d Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel (Carl Dong) 06b88ffb8ae7f2b2a93a32908cd80e77fafd270c LoadMempool: Pass in load_path, stop using gArgs (Carl Dong) b857ac60d9a0433036519c26675378bbf56a1de1 test/fuzz: Invoke LoadMempool via CChainState (Carl Dong) b3267258b052557fc136b9a4dcb754afb9219470 Move FopenFn to fsbridge namespace (Carl Dong) ae1e8e37567fa603a5977d7d05105c682dd3f7db mempool: Use NodeClock+friends for LoadMempool (Carl Dong) f9e8e5719f28d84f68f7d75e26c8e7fccac8e7d3 mempool: Improve comments for [GS]etLoadTried (Carl Dong) 813962da0b17b918941c6849996845e35d84a451 scripted-diff: Rename m_is_loaded -> m_load_tried (Carl Dong) 413f4bb52b72e082ad8716664ede48352b8e7e5a DumpMempool: Pass in dump_path, stop using gArgs (Carl Dong) bd4407817e523e3c5b347bc6be25ed007cb27034 DumpMempool: Use std::chrono instead of weird int64_t arthmetics (Carl Dong) c84390b741ab7b61c9f702d8b447c8cadc1257c8 test/mempool_persist: Test manual savemempool when -persistmempool=0 (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- This PR moves `{Dump,Load}Mempool` into its own `kernel/mempool_persist` module and introduces `ArgsManager` `node::` helpers in `node/mempool_persist_args`to remove the scattered calls to `GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)`. More context can be gleaned from the commit messages. ----- One thing I was reflecting on as I wrote this was that in the long run, I think we should probably invert the validation <-> mempool relationship. Instead of mempool not depending on validation, it might make more sense to have validation not depend on mempool. Not super urgent since `libbitcoinkernel` will include both validation and mempool, but perhaps something for the future. ACKs for top commit: glozow: re ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d via `git range-diff 7ae032e...cb3e9a1` MarcoFalke: ACK cb3e9a1e3f 🔒 ryanofsky: Code review ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Tree-SHA512: 979d7237c3abb5a1dd9b5ad3dbf3b954f906a6d8320ed7b923557f41a4472deccae3e8a6bca0018c8e7a3c4a93afecc502acd1e26756f2054f157f1c0edd939d
2022-07-15Move {Load,Dump}Mempool to kernel namespaceCarl Dong
Also: 1. Add the newly introduced kernel/mempool_persist.cpp to IWYU CI script 2. Add chrono mapping for iwyu
2022-07-15Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernelCarl Dong
It is no longer used by anything inside libbitcoinkernel, move it to node/mempool_persist_args.h where it belongs.
2022-07-15LoadMempool: Pass in load_path, stop using gArgsCarl Dong
Also: 1. Have CChainState::LoadMempool and ::ThreadImport take in paths and pass it through untouched to LoadMempool. 2. Make LoadMempool exit early if the load_path is empty. 3. Adjust the call to ::ThreadImport in ::AppInitMain to correctly pass in an empty path if mempool persistence is disabled.
2022-07-15test/fuzz: Invoke LoadMempool via CChainStateCarl Dong
Not only does this increase coverage, it is also more correct in that when ::LoadMempool is called with a mempool and chainstate, it calls AcceptToMemoryPool with just the chainstate. AcceptToMemoryPool will then act on the chainstate's mempool via CChainState::GetMempool, which may be different from the mempool originally passed to ::LoadMempool. (In this fuzz test's case, it definitely is different) Also, move DummyChainstate to its own file since it's now used by the validation_load_mempool fuzz test to replace CChainState's m_mempool.
2022-07-15Move FopenFn to fsbridge namespaceCarl Dong
[META] In a future commit in this patchset, it will be used by more than just validation, and it needs to align with fopen anyway.
2022-07-15DumpMempool: Pass in dump_path, stop using gArgsCarl Dong
Also introduce node::{ShouldPersistMempool,MempoolPath} helper functions in node/mempool_persist_args.{h,cpp} which are used by non-kernel DumpMempool callers to determine whether or not to automatically dump the mempool and where to dump it to.
2022-07-14Move ChainstateManagerOpts into kernel:: namespaceCarl Dong
It should have been there in the first place.
2022-06-29Use AutoFile where possibleMacroFake
2022-06-28mempool: Pass in -mempoolexpiry instead of referencing gArgsCarl Dong
- Store the mempool expiry (-mempoolexpiry) in CTxMemPool as a std::chrono::seconds member. - Remove the requirement to explicitly specify a mempool expiry for LimitMempoolSize(...), just use the newly-introduced member. - Remove all now-unnecessary instances of: std::chrono::hours{gArgs.GetIntArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}
2022-06-20refactor: move DEFAULT_*_LIMIT assertions from validation to policyfanquake
2022-06-20refactor: Move DEFAULT_DESCENDANT_SIZE_LIMIT to policy/policy.hfanquake
2022-06-20refactor: Move DEFAULT_DESCENDANT_LIMIT to policy/policy.hfanquake
2022-06-20refactor: Move DEFAULT_ANCESTOR_SIZE_LIMIT to policy/policy.hfanquake
2022-06-20refactor: Move DEFAULT_ANCESTOR_LIMIT to policy/policy.hCAnon
2022-06-10Merge bitcoin/bitcoin#24931: Strengthen thread safety assertionsMacroFake
ce893c0497fc9b8ab9752153dfcc77c9f427545e doc: Update developer notes (Anthony Towns) d2852917eecad6ab422a7b2c9892d351a7f0cc96 sync.h: Imply negative assertions when calling LOCK (Anthony Towns) bba87c0553780eacf0317fbfec7330ea27aa02f8 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns) a559509a0b8cade27199740212d7b589f71a0e3b sync.h: Add GlobalMutex type (Anthony Towns) be6aa72f9f8d50b6b5b19b319a74abe7ab4099ff qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns) f24bd45b37e1b2d19e5a053dbfefa30306c1d41a net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns) Pull request description: This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions. This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex` to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`). ACKs for top commit: MarcoFalke: review ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e 🐦 hebasto: ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
2022-05-31Move minRelayTxFee to policy/settingsMacroFake
Also fix includes using iwyu
2022-05-20validation: Have ChainstateManager own m_chainparamsCarl Dong
We want m_chainparams to be alive for the duration of ChainstateManager's lifetime since ChainstateManager's behaviour depends on m_chainparams. We could allow for a std::shared_ptr to be passed in as m_chainparams, but that complicates things further. Given that CChainParams is not an entity class or struct, we can just copy it and have ChainstateManager own it.
2022-05-20Add ChainstateManager::m_adjusted_time_callbackCarl Dong
This decouples validation.cpp from netaddress.cpp (transitively, timedata.cpp, and asmap.cpp). This is important for libbitcoinkernel as: - There is no reason for the consensus engine to be coupled with netaddress, timedata, and asmap - Users of libbitcoinkernel can now easily supply their own std::function that provides the adjusted time. See the src/Makefile.am changes for some satisfying removals.
2022-05-20Add ChainstateManagerOpts, using as ::OptionsCarl Dong
[META] Although it seems like we don't need it for just one option, we're going to introduce another member to this struct *in the next commit*. In future patchsets for libbitcoinkernel decoupling it from ArgsManager, even more members will be added here.