aboutsummaryrefslogtreecommitdiff
path: root/src/validation.h
AgeCommit message (Collapse)Author
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-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>
2022-02-09validation: replace lock with annotation in UnloadBlockIndex()Jon Atack
2022-02-02validation: use stronger EXCLUSIVE_LOCKS_REQUIRED()Vasil Dimov
https://github.com/bitcoin/bitcoin/pull/24103 added annotations to denote that the callers of `CChainState::ActivateBestChain()` and `CChainState::InvalidateBlock()` must not own `m_chainstate_mutex` at the time of the call. Replace the added `LOCKS_EXCLUDED()` with a stronger `EXCLUSIVE_LOCKS_REQUIRED()`, see https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative for the difference between both.
2022-01-31Merge bitcoin/bitcoin#24103: Replace RecursiveMutex `m_cs_chainstate` with ↵MarcoFalke
Mutex, and rename it 020acea99b605c9b5ee7939a6acef131db84ad4a refactor: replace RecursiveMutex m_chainstate_mutex with Mutex (w0xlt) ddeefeef20fa2fe48c3c4563370a6297704d228e refactor: add negative TS annotations for `m_chainstate_mutex` (w0xlt) 1dfd31bc267c54144a7e62ad5a1a5860c032f4d7 scripted-diff: rename m_cs_chainstate -> m_chainstate_mutex (w0xlt) Pull request description: This PR is related to #19303 and gets rid of the `RecursiveMutex m_cs_chainstate`. `m_cs_chainstate` is only held in `ActivateBestChain()` and `InvalidateBlock()`. So apparently there is no recursion involved, so the `m_cs_chainstate` can be a non-recursive mutex. ACKs for top commit: hebasto: ACK 020acea99b605c9b5ee7939a6acef131db84ad4a, I have reviewed the code and it looks OK, I agree it can be merged. theStack: Code-review ACK 020acea99b605c9b5ee7939a6acef131db84ad4a 🌴 shaavan: reACK 020acea99b605c9b5ee7939a6acef131db84ad4a Tree-SHA512: c7c16e727e326df3410514915ce753a2a5e1da78857ef965ef683e36251e1b73c9cced4cd5231b04dbe2be0ea14084f6731b4d7a4d9a8e086e982b985e37e4b4
2022-01-27scripted-diff: Clarify CheckFinalTxAtTip nameMarcoFalke
This checks finality at the current Tip, so clarify this in its name. -BEGIN VERIFY SCRIPT- ren() { sed -i "s/\<$1\>/$2/g" $( git grep -l "$1" ./src/ ) ; } ren CheckSequenceLocks CheckSequenceLocksAtTip ren CheckFinalTx CheckFinalTxAtTip -END VERIFY SCRIPT-
2022-01-27policy: Remove unused locktime flagsMarcoFalke
2022-01-25Merge bitcoin/bitcoin#23804: validation: followups for de-duplication of ↵fanquake
packages 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 [unit test] package parents are a mix (glozow) de075a98eaf0b3f7676c5c78b50b66902202b34c [validation] better handle errors in SubmitPackage (glozow) 9d88853e0c85f765f7d982b15e8122ede50110ed AcceptPackage fixups (glozow) 2db77cd3b835d052de678755bcdde5a645ce2d65 [unit test] different witness in package submission (glozow) 9ad211c5753dbd148ba6f0ed56854f6364362ca8 [doc] more detailed explanation for deduplication (glozow) 83d4fb71260f268abd41d083fb3458476aed83ce [packages] return DIFFERENT_WITNESS for same-txid-different-witness tx (glozow) Pull request description: This addresses some comments from review on e12fafda2dfbbdf63f125e5af797ecfaa6488f66 from #22674. - Improve documentation about de-duplication: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770156708) - Fix code looking up same-txid-different-witness transaction in mempool: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770804029) - Improve the interface for when a same-txid-different-witness transaction is swapped: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770782822) - Add a test for witness swapping: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770804029) - Add a test for packages with a mix of duplicate/different witness/new parents: [comment](https://github.com/bitcoin/bitcoin/pull/22674#discussion_r773037608) - Fix issue with not notifying `CValidationInterface` when there's a partial submission due to fail-fast: [comment](https://github.com/bitcoin/bitcoin/pull/22674#discussion_r773013162) ACKs for top commit: achow101: ACK 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 t-bast: LGTM, ACK https://github.com/bitcoin/bitcoin/pull/23804/commits/3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 instagibbs: ACK 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 ariard: ACK 3cd7f69 Tree-SHA512: a5d86ca86edab80a5a05fcbb828901c058b3f2fa2552912ea52f2871e29c3cf4cc34020e7aac2217959c9c3a01856f4bd3d631d844635b98144f212f76c2f3ef
2022-01-24refactor: replace RecursiveMutex m_chainstate_mutex with Mutexw0xlt
2022-01-24refactor: add negative TS annotations for `m_chainstate_mutex`w0xlt
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2022-01-19scripted-diff: rename m_cs_chainstate -> m_chainstate_mutexw0xlt
-BEGIN VERIFY SCRIPT- s() { sed -i 's/m_cs_chainstate/m_chainstate_mutex/g' $1; } s src/validation.cpp s src/validation.h -END VERIFY SCRIPT-
2022-01-17[packages] return DIFFERENT_WITNESS for same-txid-different-witness txglozow
The previous interface required callers to guess that the tx had been swapped and look up the tx again by txid to find a `MEMPOOL_ENTRY` result. This is a confusing interface. Instead, explicitly tell the caller that this transaction was `DIFFERENT_WITNESS` in the result linked to the mempool entry's wtxid. This gives the caller all the information they need in 1 lookup, and they can query the mempool for the other transaction if needed.
2022-01-11Remove cs_main lock annotation from ChainstateManager.m_blockmanRyan Ofsky
BlockManager is a large data structure, and cs_main is not required to take its address or access every part of it. Individual BlockManager fields and methods which do require cs_main like m_block_index and LookupBlockIndex are already annotated separately, and these other annotations describe locking requirements more accurately and do a better job enforcing thread safety. Since cs_main is not needed to access the address of the m_block object, this commit drops cs_main LOCK calls which were added pointlessly to satisfy this annotation in the past. Co-authored-by: Carl Dong <contact@carldong.me>
2022-01-06Add src/node/* code to node:: namespaceRussell Yanofsky
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-15 12:19:25 +0000