aboutsummaryrefslogtreecommitdiff
path: root/src/validation.h
AgeCommit message (Collapse)Author
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.
2022-05-21scripted-diff: Convert global Mutexes to GlobalMutexesAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i -E -e '/^([a-z]+ )?Mutex [a-z]/ s/Mutex/GlobalMutex/' $(git grep -lE '^([a-z]+ )?Mutex [a-z]') -END VERIFY SCRIPT-
2022-05-10validation: move g_versionbitscache into ChainstateManagerAnthony Towns
2022-05-10deploymentstatus: allow chainman in place of consensusParamsAnthony Towns
2022-05-10validation: move UpdateUncommittedBlockStructures and ↵Anthony Towns
GenerateCoinbaseCommitment into ChainstateManager
2022-05-10validation: replace ::Params() calls with chainstate/chainman memberAnthony Towns
2022-05-10validation: remove redundant CChainParams params from ChainstateManager methodsAnthony Towns
2022-05-10validation: add CChainParams to ChainstateManagerAnthony Towns
2022-05-03Merge bitcoin/bitcoin#24470: Disallow more unsafe string->path conversions ↵MacroFake
allowed by path append operators f64aa9c411ad78259756a28756ec1eb8069b5ab4 Disallow more unsafe string->path conversions allowed by path append operators (Ryan Ofsky) Pull request description: Add more `fs::path` `operator/` and `operator+` overloads to prevent unsafe string->path conversions on Windows that would cause strings to be decoded according to the current Windows locale & code page instead of the correct string encoding. Update application code to deal with loss of implicit string->path conversions by calling `fs::u8path` or `fs::PathFromString` explicitly, or by just changing variable types from `std::string` to `fs::path` to avoid conversions altogether, or make them happen earlier. In all cases, there's no change in behavior either (1) because strings only contained ASCII characters and would be decoded the same regardless of what encoding was used, or (2) because of the 1:1 mapping between paths and strings using the `PathToString` and `PathFromString` functions. Motivation for this PR was just that I was experimenting with #24469 and noticed that operations like `fs::path / std::string` were allowed, and I thought it would be better not to allow them. ACKs for top commit: hebasto: ACK f64aa9c411ad78259756a28756ec1eb8069b5ab4 Tree-SHA512: 944cce49ed51537ee7a35ea4ea7f5feaf0c8fff2fa67ee81ec5adebfd3dcbaf41b73eb35e49973d5f852620367f13506fd12a7a9b5ae3a7a0007414d5c9df50f
2022-04-28validation: make CScriptCheck and prevector swap member functions noexceptJon Atack
Reason: A swap must not fail; when a class has a swap member function, it should be declared noexcept. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c84-a-swap-function-must-not-fail
2022-04-27validation: Prune UnloadBlockIndex and calleesCarl Dong
In previous commits in this patchset, we've made sure that every Unload/UnloadBlockIndex member function resets its own members, and does not reach out to globals. This means that their corresponding classes' default destructors can now replace them, and do an even more thorough job without the need to be updated for every new member variable. Therefore, we can remove them, and also remove UnloadBlockIndex since that's not used anymore. Unfortunately, chainstatemanager_loadblockindex relies on CChainState::UnloadBlockIndex, so that needs to stay for now.
2022-04-27validation: No mempool clearing in UnloadBlockIndexCarl Dong
The only caller that uses this is ~ChainTestingSetup() where we immediately destroy the mempool afterwards.
2022-04-27Clear {versionbits,warning}cache in ~ChainstatemanagerCarl Dong
Also add TODO item to deglobalize the {versionbits,warning}cache, which should really only need to be cleared if we change the chainparams.
2022-04-26validation: default initialize and guard chainman membersAnthony Towns
2022-04-26Merge bitcoin/bitcoin#24917: Make BlockManager::LoadBlockIndex privatefanquake
fa1970f075292d7312654730a994a68c2ca8bc06 Make BlockManager::LoadBlockIndex private (MarcoFalke) Pull request description: * After commit fa27f03b4943540aa2eab283d4cf50ad4a1a01f8 `BlockManager::LoadBlockIndex` is only called by `BlockManager::LoadBlockIndexDB`. Thus, it can be made `private`. * After commit c600ee38168a460d3026eae0e289c976194aad14 `m_best_invalid` is no longer accessed by `BlockManager::LoadBlockIndex`. Thus, the unused `friend` can be removed. ACKs for top commit: mruddy: ACK fa1970f075292d7312654730a994a68c2ca8bc06 I verified by double checking references, then applying the patch, and running `make check`. LGTM. Tree-SHA512: 9b36b4c59bf7ad01171764ce61b1be9750fc92d105c4fe939b1a6a70027ab6300d5d2a2fc3e82f981e22c3987f2ca84e092d2e1f8463fa320af9f05048580c0a
2022-04-21Disallow more unsafe string->path conversions allowed by path append operatorsRyan Ofsky
Add more fs::path operator/ and operator+ overloads to prevent unsafe string->path conversions on Windows that would cause strings to be decoded according to the current Windows locale & code page instead of the correct string encoding. Update application code to deal with loss of implicit string->path conversions by calling fs::u8path or fs::PathFromString explicitly, or by just changing variable types from std::string to fs::path to avoid conversions altoghther, or make them happen earlier. In all cases, there's no change in behavior either (1) because strings only contained ASCII characters and would be decoded the same regardless of what encoding was used, or (2) because of the 1:1 mapping between paths and strings using the PathToString and PathFromString functions. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2022-04-19scripted-diff: Rename pindexBestHeader, fHavePrunedCarl Dong
...to m_best_header and m_have_pruned -BEGIN VERIFY SCRIPT- find_regex="\bpindexBestHeader\b" \ && git grep -l -E "$find_regex" -- src \ | xargs sed -i -E "s@$find_regex@m_best_header@g" find_regex="\bfHavePruned\b" \ && git grep -l -E "$find_regex" -- src \ | xargs sed -i -E "s@$find_regex@m_have_pruned@g" -END VERIFY SCRIPT-
2022-04-19move-mostly: Make pindexBestHeader a ChainMan memberCarl Dong
[META] In the next commit, we move the clearing of pindexBestHeader to ChainstateManager::Unload()
2022-04-19Make BlockManager::LoadBlockIndex privateMarcoFalke
2022-04-05[packages/policy] use package feerate in package validationglozow
This allows CPFP within a package prior to submission to mempool.
2022-03-17Merge bitcoin/bitcoin#24515: Only load BlockMan in BlockMan member functionsMarcoFalke
f865cf8ded2b2fbc82a6fbc41226d991909a6880 Add and use BlockManager::GetAllBlockIndices (Carl Dong) 28ba0313eac37e4a900b7e97af7169ce999c4024 Add and use CBlockIndexHeightOnlyComparator (Carl Dong) 12eb05df63f930969115af6dc66e2e5d02f2a517 move-only: Move CBlockIndexWorkComparator to blockstorage (Carl Dong) c600ee38168a460d3026eae0e289c976194aad14 Only load BlockMan in BlockMan member functions (Carl Dong) 42e56d9b188f97c077ed2269e24acc0be35ece17 style-only: No need for std::pair for vSortedByHeight (Carl Dong) 3bbb6fea051f4e19bd2448e401a6c4e9b4cc7a41 style-only: Various blockstorage.cpp cleanups (Carl Dong) 5be9ee3c54dcb396ff52fc8c8b7e4e6e39ec4a3b refactor: more const annotations for uses of CBlockIndex* (Anthony Towns) Pull request description: The only important commit is "Only load BlockMan in BlockMan member functions", everything else is all just small style changes. Here's the commit message, reproduced: ``` This commit effectively splits the "load block index itself" logic from "derive Chainstate variables from loaded block index" logic. This means that BlockManager::LoadBlockIndex{,DB} will only load what's relevant to the BlockManager. ``` ACKs for top commit: ajtowns: ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 ; code review only MarcoFalke: review ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 🗂 Tree-SHA512: 7b204d782834e06fd7329d022e2ae860181b4e8105c33bfb928539a4ec24161dc7438a9c4d4ee279dcad77de310c160b997bb8aa18923243d0fd55ccf4ad7c3a
2022-03-15Only load BlockMan in BlockMan member functionsCarl Dong
This commit effectively splits the "load block index itself" logic from "derive Chainstate variables from loaded block index" logic. This means that BlockManager::LoadBlockIndex{,DB} will only load what's relevant to the BlockManager. I strongly recommend reviewing with the following git-diff flags: --color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
2022-03-14Clarify that CheckSequenceLocksAtTip is a validation functionMarcoFalke
2022-03-14Merge bitcoin/bitcoin#24080: policy: Remove unused locktime flagsMarcoFalke
fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 scripted-diff: Clarify CheckFinalTxAtTip name (MarcoFalke) fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35 policy: Remove unused locktime flags (MarcoFalke) Pull request description: The locktime flags have many issues: * They are passed in by a default argument, which is fragile. It has already lead to bugs like the one fixed in commit e30b6ea194fee3bb95a45e7b732a99566b88f1f5. * They are negative (signed), which doesn't make sense for flags (unsigned in general). According to the review comments when the code was added: "The max on the flags is a fairly weird operation." (https://github.com/bitcoin/bitcoin/pull/6566#issuecomment-150310861) * No call site relies on the default argument and they all pass in a single compile-time constant, rendering most of the code dead and untested. * The dead code calls `GetAdjustedTime` (network adjusted time), which has its own issues. See https://github.com/bitcoin/bitcoin/issues/4521 Fix all issues by removing them ACKs for top commit: ajtowns: ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 theStack: Code-review ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 glozow: ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1, agree the default arg `flags` is a massive footgun and just setting max flags is weird. Adding `AtTip` to the names makes sense to me, since they're both testing for *next* block and only ever used for {,re}addition to mempool. Tree-SHA512: 79f4a52f34909eb598d88bbae7afe8abe5f85f45c128483d16aa83dacd0e5579e561b725d01b1e9a931d1821012a51ad2bc6fb2867f8d09ee541f9d234d696f8
2022-03-09refactor: more const annotations for uses of CBlockIndex*Anthony Towns
2022-03-07Merge bitcoin/bitcoin#24299: validation, refactor: UnloadBlockIndex and ↵laanwj
ChainstateManager::Reset thread safety cleanups ae9ceed3e23288b163b7d7b1840b06b8d332f4ce validation, refactoring: remove ChainstateManager::Reset() (Jon Atack) daad0093e3d1466789d0ce687902636c80cd74a1 validation: replace lock with annotation in UnloadBlockIndex() (Jon Atack) Pull request description: Thread safety refactoring seen in #24177: - replace re-acquiring lock cs_main with a thread safety annotation in UnloadBlockIndex() - remove ChainstateManager::Reset(), as it is currently unused (can be reintroduced in the test utilities if needed for unit testing) ACKs for top commit: laanwj: Code review ACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce vasild: ACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce klementtan: crACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce Tree-SHA512: cebb782572997cc2dda01590d6bb6c5e479e8202324d8b6ff459b814ce09e818b996c881736bfebd1b8bf4b6d7a0f79faf3ffea176a4699dd7d7429de2db2d13
2022-02-21Merge bitcoin/bitcoin#24347: rpc: Fix implicit-integer-sign-change in ↵MarcoFalke
verifychain fa8dad0e078c577d740a9667636733957586c035 rpc: Fix implicit-integer-sign-change in verifychain (MarcoFalke) Pull request description: It doesn't really make sense to treat `DEFAULT_CHECKLEVEL` as unsigned as long as `VerifyDB` accepts a signed integer. Making it signed also avoids a cast round trip from signed->unsigned->signed in the RPC. ACKs for top commit: luke-jr: utACK fa8dad0e078c577d740a9667636733957586c035 theStack: Code-review ACK fa8dad0e078c577d740a9667636733957586c035 Tree-SHA512: 75499dbe4ace2962792e5fbec7defb10c25fdbbfde951d5e542a91daa880cc50395da0287173e2c84a28e18267c74af7b44b9f38ce364bcb0216c402f65b7641
2022-02-15rpc: Fix implicit-integer-sign-change in verifychainMarcoFalke
2022-02-09Add missing thread safety lock assertions in validation.hJon Atack
2022-02-09validation, refactoring: remove ChainstateManager::Reset()Jon Atack
Co-authored-by: Vasil Dimov <vd@FreeBSD.org> Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>