aboutsummaryrefslogtreecommitdiff
path: root/src/test/miner_tests.cpp
AgeCommit message (Collapse)Author
2023-05-20refactor: Move system from util to common libraryTheCharlatan
Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md.
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-06Move random test util code from setup_common to randomJon Atack
as many of the unit tests don't use this code
2023-01-31refactor: Move calculation logic out from `CheckSequenceLocksAtTip()`Hennadii Stepanov
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-19Merge bitcoin/bitcoin#25311: refactor: remove CBlockIndex copy constructionfanquake
36c201feb74bbb87d22bd956373dbbb9c47fb7e7 remove CBlockIndex copy construction (James O'Beirne) Pull request description: Copy construction of CBlockIndex objects is a footgun because of the wide use of equality-by-pointer comparison in the code base. There are also potential lifetime confusions of using copied instances, since there are recursive pointer members (e.g. pprev). (See also https://github.com/bitcoin/bitcoin/pull/24008#discussion_r891949166) We can't just delete the copy constructors because they are used for derived classes (CDiskBlockIndex), so we mark them protected. ACKs for top commit: ajtowns: ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 - code review only MarcoFalke: re-ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 🏻 Tree-SHA512: b1cf9a1cb992464a4377dad609713eea63cc099435df374e4553bfe62d362a4eb5e3c6c6649177832f38c0905b23841caf9d62196cef8e3084bfea0bfc26374b
2022-12-15remove CBlockIndex copy constructionJames O'Beirne
Copy construction of CBlockIndex objects is a footgun because of the wide use of equality-by-pointer comparison in the code base. There are also potential lifetime confusions of using copied instances, since there are recursive pointer references (e.g. pprev). We can't just delete the copy constructors because they are used for derived classes (CDiskBlockIndex), so we mark them protected. Delete move constructors and declare the destructor to satisfy the "rule of 5."
2022-10-24test: Use type-safe NodeSeconds for TestMemPoolEntryHelperMacroFake
2022-10-18test: Remove unused txmempool include from testsMacroFake
2022-10-05test: Use dedicated mempool in TestBasicMiningMacroFake
No need for a shared mempool. Also remove unused chainparams parameter. Can be reviewed with --ignore-all-space
2022-10-05test: Use dedicated mempool in TestPackageSelectionMacroFake
No need for a shared mempool. Also remove unused chainparams parameter.
2022-10-05test: Use dedicated mempool in TestPrioritisedMiningMacroFake
No need for a shared mempool. Also remove unused chainparams parameter.
2022-10-05test: Pass mempool reference to AssemblerForTestMacroFake
2022-10-04test: Remove unused fCheckpointsEnabled from miner_testsMacroFake
The earliest checkpoint is at height 11111, so this can't possibly have any impact on this test.
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-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-03Remove unused SetTip(nullptr) codeMacroFake
2022-07-22refactor: make active_chain_tip a referenceAurèle Oulès
2022-06-22Remove LOCKTIME_MEDIAN_TIME_PAST constantMarcoFalke
2022-06-06miner: Make mempool optional for BlockAssemblerCarl Dong
...also adjust callers Changes: - In BlockAssembler::CreateNewBlock, we now only lock m_mempool->cs and call addPackageTxs if m_mempool is not nullptr - BlockAssembler::addPackageTxs now takes in a mempool reference, and is annotated to require that mempool's lock. - In TestChain100Setup::CreateBlock and generateblock, don't construct an empty mempool, just pass in a nullptr for mempool
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-18Do not pass CChainParams& to BlockAssembler constructorMacroFake
2022-05-10validation: remove redundant CChainParams params from ChainstateManager methodsAnthony Towns
2022-05-06Merge bitcoin/bitcoin#24804: Sanity assert GetAncestor() != nullptr where ↵MacroFake
appropriate 308dd2e93e92f4cac4e7d75478316af9bb2b77b8 Sanity assert GetAncestor() != nullptr where appropriate (Adam Jonas) Pull request description: Re-opening #17232. I have rebased the PR and addressed jonatack's nit suggestions. Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate. In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time. In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior. Co-Authored-By: Adam Jonas <jonas@chaincode.com> Co-Authored-By: danra <danra@users.noreply.github.com> ACKs for top commit: jonatack: ACK 308dd2e93e92f4cac4e7d75478316af9bb2b77b8 Tree-SHA512: 5bfdaab1499607ae2c3cd3e2e9e8c37850bfd0e327e680f4e36c81f9c6d98a543af78ecfac1ab0e06325d264412615a04d52005875780c7db2a4d81bd2d2259a
2022-05-05Sanity assert GetAncestor() != nullptr where appropriateAdam Jonas
Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate. In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time. In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior. Co-Authored-By: Aurèle Oulès <aurele@oules.com> Co-Authored-By: danra <danra@users.noreply.github.com>
2022-03-14[unit test] prioritisation in miningglozow
2022-03-14MOVEONLY: group miner tests into MinerTestingSetup functionsglozow
No behavior changes. Recommend using --color-moved=dimmed_zebra.
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-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-26Extract CTxIn::MAX_SEQUENCE_NONFINAL constantMarcoFalke
2022-01-06Add src/node/* code to node:: namespaceRussell Yanofsky
2021-11-16scripted-diff: Move miner to src/nodeMarcoFalke
-BEGIN VERIFY SCRIPT- # Move module git mv src/miner.cpp src/node/ git mv src/miner.h src/node/ # Replacements sed -i 's:miner\.h:node/miner.h:g' $(git grep -l miner) sed -i 's:miner\.cpp:node/miner.cpp:g' $(git grep -l miner) sed -i 's:MINER_H:NODE_MINER_H:g' $(git grep -l MINER_H) -END VERIFY SCRIPT-
2021-08-05test: Add missing includeMarcoFalke
2021-06-18test: Properly set BIP34 height in CreateNewBlock_validity unit testMarcoFalke
2021-06-10scripted-diff: test: Use existing chainman in unit testsCarl Dong
-BEGIN VERIFY SCRIPT- git ls-files -- src/test \ | grep -v '^src/test/fuzz' \ | xargs sed -i -E \ -e 's@g_chainman\.m_blockman@m_node.chainman->m_blockman@g' \ -e 's@([^:])(Chain(state|)Active)@\1::\2@g' \ -e 's@::Chain(state|)Active\(\)@m_node.chainman->ActiveChain\1()@g' -END VERIFY SCRIPT-
2021-06-10test/miner_tests: Pass in chain tip to CreateBlockIndexCarl Dong
2021-06-02[refactor] comment/naming improvementsglozow
2021-05-20[validation] make CheckSequenceLocks context-freeglozow
Allow CheckSequenceLocks to use heights and coins from any CoinsView and CBlockIndex provided. This means that CheckSequenceLocks() doesn't need to hold the mempool lock or cs_main. The caller is responsible for ensuring the CoinsView and CBlockIndex are consistent before passing them in. The typical usage is still to create a CCoinsViewMemPool from the mempool and grab the CBlockIndex from the chainstate tip.
2021-03-24miner: Add chainstate member to BlockAssemblerCarl Dong
2021-03-24Revert "scripted-diff: Invoke CreateNewBlock with chainstate"Carl Dong
This reverts commit 46b7f29340acb399fbd2378508a204d8d8ee8fca.
2021-03-08scripted-diff: Invoke CreateNewBlock with chainstateCarl Dong
-BEGIN VERIFY SCRIPT- find_regex='(\.|->)CreateNewBlock\(' \ && git grep -l -E "$find_regex" -- src \ | grep -v '^src/miner\.\(cpp\|h\)$' \ | xargs sed -i -E 's@'"$find_regex"'@\0::ChainstateActive(), @g' -END VERIFY SCRIPT-
2021-02-20Merge #20750: [Bundle 2/n] Prune g_chainman usage in mempool-related ↵MarcoFalke
validation functions e8ae1db864b09a47c736631e6cd3f5ec17929850 style-only: Make AcceptToMemoryPool signature readable (Carl Dong) 8f5c100064bea720351d450f8116ff3abe0515cc style-only: Make CheckSequenceLock signature readable (Carl Dong) 8c824819c85005ee6c783e9f8fa43ff91716e33d validation: Use *this in CChainState::LoadMempool (Carl Dong) 0a9a24d8c717e88e36e16014630cec8eada8dfcb validation: Pass in chainstate to UpdateMempoolForReorg (Carl Dong) 714201881251a787423fbca34f70fed505e9dc28 validation: Pass in chainstate to CTxMemPool::removeForReorg (Carl Dong) 71734c65dc491a4bb654ccbb7a1dd0e12131cee4 validation: Pass in chain to ::TestLockPointValidity (Carl Dong) 120aaba9ac41af71a760aa0969dd090e96786fb3 tree-wide: Fix erroneous AcceptToMemoryPool replacements (Carl Dong) 417dafc1ee07af3319c2fe89758123cb8362ff16 validation: Remove old AcceptToMemoryPool w/o chainstate param (Carl Dong) 3704433c4f5ecf9f196860b2ccecae0d2c8b5f6e scripted-diff: Invoke ::AcceptToMemoryPool with chainstate (Carl Dong) 229bc37b5f18cffbc85efbad3b6e9047c6951e95 validation: Pass in chainstate to ::AcceptToMemoryPool (Carl Dong) d0da7ea57ab932eca956458fb3633585ff3c0003 validation: Pass in chainstate to ::LoadMempool (Carl Dong) 3a205c43dc03cc833daba93087279402f640965b validation: Pass in chainstate to AcceptToMemoryPoolWithTime (Carl Dong) d8a816329c878b5973d28d370c0f64ebbdde716b validation: Add chainstate member to MemPoolAccept (Carl Dong) 4c15942b79c46256950df17c348302679e668ebc validation: Pass in chainstate to ::CheckSequenceLocks (Carl Dong) 577b774d0c664b891bc9e1550ef179a655a466ad validation: Remove old CheckFinalTx w/o chain tip param (Carl Dong) 7031cf89db943d3e73597d2f9fa4a41908558e6c scripted-diff: Invoke ::CheckFinalTx with chain tip (Carl Dong) d015eaa550027a387cd548cf0bcfa1a4c31a3374 validation: Pass in chain tip to ::CheckFinalTx (Carl Dong) 252b489c9f9c9e7dceb919e9cbd208ea72d75e68 validation: Pass in coins tip to CheckInputsFromMempoolAndCache (Carl Dong) 73a6d2b7bea832fe24870dd7593c8fc1028e8d57 validation: Pass in chainstate to IsCurrentForFeeEstimation (Carl Dong) d1f932b0b0685690e5142272a2ed6a21237fbf05 validation: Pass in coins cache to ::LimitMempoolSize (Carl Dong) Pull request description: Overall PR: #20158 (tree-wide: De-globalize ChainstateManager) Note to reviewers: 1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so: 1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only** 2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase** 3. Remove `old_function` ACKs for top commit: glozow: reACK https://github.com/bitcoin/bitcoin/commit/e8ae1db864b09a47c736631e6cd3f5ec17929850 via `git range-diff 15f0042...e8ae1db`, only change is fixing ATMP call from conflict MarcoFalke: ACK e8ae1db864b09a47c736631e6cd3f5ec17929850 📣 Tree-SHA512: 6af50f04940a69c5c3d3796a24f32f963fa02503cdc1155cc11fff832a99172b407cd163a19793080a5af98580f051b48195b62ec4a797ba2763b4883174153d
2021-02-18validation: Pass in chainstate to ::CheckSequenceLocksCarl Dong
2021-02-18scripted-diff: Invoke ::CheckFinalTx with chain tipCarl Dong
-BEGIN VERIFY SCRIPT- find_regex='\bCheckFinalTx\(' \ && git grep -l -E "$find_regex" -- src \ | grep -v '^src/validation\.\(cpp\|h\)$' \ | xargs sed -i -E 's@'"$find_regex"'@\0::ChainActive().Tip(), @g' -END VERIFY SCRIPT-
2021-02-18Avoid comparision of integers with different signsJonas Schnelli
2021-02-18Merge #20429: refactor: replace (sizeof(a)/sizeof(a[0])) with C++17 std::sizeMarcoFalke
e829c9afbf75e930db6c3fe77a269b0af5e7a3ad refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner) 365539c84691d470b44d35df374d8c049f8c1192 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner) 63d4ee1968144cc3d115f92baef95785abf813ac refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner) Pull request description: This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size) (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete. As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage: * all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)). * `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)). ACKs for top commit: practicalswift: cr ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad: patch looks correct fanquake: ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad MarcoFalke: review ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad 🌩 Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d
2021-02-16[test] Throw error instead of segfaulting in failure scenarioAmiti Uttarwar
If the miner code is faulty and does not include any transactions in a block, the code segfaults when it tries to access block transactions. Instead, add a check that safely aborts the process.
2021-01-31refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17)Sebastian Falbesoner
Removes the macro ARRAYLEN and also substitutes all other uses of the same "sizeof(a)/sizeof(a[0])" pattern by std::size, available since C++17.
2021-01-31refactor: iterate arrays via C++11 range-based for loops if idx is not neededSebastian Falbesoner
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-09 09:27:38 +0000