aboutsummaryrefslogtreecommitdiff
path: root/src/txmempool.cpp
AgeCommit message (Collapse)Author
2022-06-28mempool: Use m_limit for UpdateTransactionsFromBlockCarl Dong
Since: - UpdateTransactionsFromBlock is only called by MaybeUpdateMempoolForReorg, which calls it with the gArgs-determined ancestor limits - UpdateForDescendants is only called by UpdateTransactionsFromBlock with the ancestor limits unchanged We can remove the requirement to specify the ancestor limits for both UpdateTransactionsFromBlock and UpdateForDescendants and just use the values in the m_limits member. Also move some removed comments to MemPoolLimits struct members. The uint64_t cast in UpdateForDescendants is not new behavior, see the diff in CChainState::MaybeUpdateMempoolForReorg for where they were previously.
2022-06-28mempool: Introduce (still-unused) MemPoolLimitsCarl Dong
They live as a CTxMemPool member. [META] These limits will be used in subsequent commits to replace calls to gArgs.
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-28mempool: Pass in -maxmempool instead of referencing gArgsCarl Dong
- Store the mempool size limit (-maxmempool) in CTxMemPool as a member. - Remove the requirement to explicitly specify a mempool size limit for CTxMemPool::GetMinFee(...) and LimitMempoolSize(...), just use the stored mempool size limit where possible. - Remove all now-unnecessary instances of: gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE_MB) * 1000000 The code change in CChainState::GetCoinsCacheSizeState() is correct since the coinscache should not repurpose "extra" mempool memory headroom for itself if the mempool doesn't even exist.
2022-06-28pool: Add and use MemPoolOptions, ApplyArgsManOptionsCarl Dong
Reviewers: Note that CTxMemPool now requires a non-defaulted CTxMemPool::Options for its constructor. Meaning that there's no need to worry about a stray CTxMemPool constructor somewhere defaulting to something incorrect. All instances of CTxMemPool construction are addressed here in this commit. We set options for CTxMemPool and construct it in many different ways. A good example can be seen in how we determine CTxMemPool's check_ratio in AppInitMain(...). 1. We first set the default based on chainparams's DefaultConsistencyChecks() 2. Then, we apply the ArgsManager option on top of that default 3. Finally, we clamp the result of that between 0 and 1 Million With this patch, most CTxMemPool construction are along the lines of: MemPoolOptions mempool_opts{...default overrides...}; ApplyArgsManOptions(argsman, mempool_opts); ...hard overrides... CTxMemPool pool{mempool_opts}; This "compositional" style of building options means that we can omit unnecessary/irrelevant steps wherever we want but also maintain full customizability. For example: - For users of libbitcoinkernel, where we eventually want to remove ArgsManager, they simply won't call (or even know about) ApplyArgsManOptions. - See src/init.cpp to see how the check_ratio CTxMemPool option works after this change. A MemPoolOptionsForTest helper was also added and used by tests/fuzz tests where a local CTxMemPool needed to be created. The change in src/test/fuzz/tx_pool.cpp seemingly changes behaviour by applying ArgsManager options on top of the CTxMemPool::Options defaults. However, in future commits where we introduce flags like -maxmempool, the call to ApplyArgsManOptions is actually what preserves the existing behaviour. Previously, although it wasn't obvious, our CTxMemPool would consult gArgs for flags like -maxmempool when it needed it, so it already relied on ArgsManager information. This patchset just laid bare the obfuscatory perils of globals. [META] As this patchset progresses, we will move more and more CTxMemPool-relevant options into MemPoolOptions and add their ArgsMan-related logic to ApplyArgsManOptions.
2022-06-22Fix signed integer overflow in prioritisetransaction RPCMarcoFalke
2022-06-22refactor: Replace feeDelta by m_modified_feeMarcoFalke
* feeDelta tracked the delta (to be applied on top of the actual fee) * m_modified_fee tracks the actual fee with the delta included * Instead of passing in the new total delta to the Updater, pass in by how much the total delta should be modified. This is needed for the next commit, but makes sense on its own because the same is done by UpdateDescendantState and UpdateAncestorState.
2022-04-14Merge bitcoin/bitcoin#23416: doc: Remove fee delta TODO from txmempool.cpplaanwj
fa32cc0682a0aa3420e6a11031721fcb6c50fa44 doc: Remove fee delta TODO from txmempool.cpp (MarcoFalke) Pull request description: This refactor request was added in commit eb306664e786ae43d539fde66f0fbe2a3e89d910, though it didn't explain why the refactor is needed and what the goal is. Given that this wasn't touched for more than 5 years, it doesn't seem critical. Generally, non-trivial `TODO`s make more sense as GitHub issues, so that they can be discussed and triaged more easily. ACKs for top commit: laanwj: Code review ACK fa32cc0682a0aa3420e6a11031721fcb6c50fa44 Tree-SHA512: 6629fef543e815136c82c38aa8ba2c4de68a5fe94c6954f2559e468f7e59052e02dd7c221d3b159be0314eaf0dbb18f74814297c58f76e2289c47e8d4f49be4e
2022-04-06Merge bitcoin/bitcoin#24145: mempool: Clear vTxHashes when mapTx is clearedlaanwj
9d65ad365c539557e969a35d22723d92e0b422fe Clear vTxHashes when mapTx is cleared (Peter Bushnell) Pull request description: vTxHashes is a vector of all entries in mapTx, if you clear one you should clear the other, lest someone try to use the txiter in vTxHashes which would result in a segfault. ACKs for top commit: laanwj: Code review ACK 9d65ad365c539557e969a35d22723d92e0b422fe Tree-SHA512: 6832755e43ab7f528b46817aeadcb6ffdc965b97f59ab96bb053dedbb7e68155ba3db52286355dca33b509237f80eda249760b26db493762bc50d8e2cef16d8f
2022-04-04refactor: fix clang-tidy named args usagefanquake
2022-03-21Use CAmount for fee delta and modified feeMarcoFalke
2022-03-21Replace struct update_fee_delta with lambdaMarcoFalke
2022-01-31Clear vTxHashes when mapTx is clearedPeter Bushnell
2022-01-25Merge bitcoin/bitcoin#21464: Mempool Update Cut-Through Optimizationfanquake
c5b36b1c1b11f04e5da7fb44183f61d09a14e40d Mempool Update Cut-Through Optimization (Jeremy Rubin) c49daf9885e86ba08acdc8332d2a34bc5951a487 [TESTS] Increase limitancestorcount in tournament RPC test to showcase improved algorithm (Jeremy Rubin) Pull request description: Often when we're updating mempool entries we update entries that we ultimately end up removing the updated entries shortly thereafter. This patch makes it so that we filter for such entries a bit earlier in processing, which yields a mild improvement for these cases, and is negligible overhead otherwise. There's potential for a better -- but more sophisticated -- algorithm that can be used taking advantage of epochs, but I figured it is better to do something that is simple and works first and upgrade it later as the other epoch mempool work proceeds as it makes the patches for the epoch algorithm simpler to understand, so you can consider this as preparatory work. It could either go in now if it is not controversial, or we could wait until the other patch is ready to go. ACKs for top commit: instagibbs: reACK c5b36b1 sipa: utACK c5b36b1c1b11f04e5da7fb44183f61d09a14e40d mzumsande: Code Review ACK c5b36b1c1b11f04e5da7fb44183f61d09a14e40d Tree-SHA512: 78b16864f77a637d8a68a65e23c019a9757d8b2243486728ef601d212ae482f6084cf8e69d810958c356f1803178046e4697207ba40d6d10529ca57de647fae6
2022-01-03Merge bitcoin/bitcoin#23683: bug fix: valid but different LockPoints after a ↵MarcoFalke
reorg b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b [bugfix] update lockpoints correctly during reorg (glozow) b6002b07a36f0d58dc6becd04bfcf78599056b7c MOVEONLY: update_lock_points to txmempool.h (glozow) Pull request description: I introduced a bug in #22677 (sorry! :sweat_smile:) Mempool entries cache `LockPoints`, containing the first height/blockhash/`CBlockIndex*` at which the transaction becomes valid. During a reorg, we re-check timelocks on all mempool entries using `CheckSequenceLocks(useExistingLockPoints=false)` and remove any now-invalid entries. `CheckSequenceLocks()` also mutates the `LockPoints` passed in, and we update valid entries' `LockPoints` using `update_lock_points`. Thus, `update_lock_points(lp)` needs to be called right after `CheckSequenceLocks(lp)`, otherwise we lose the data in `lp`. I incorrectly assumed they could be called in separate loops. The incorrect behavior introduced is: if we have a reorg in which a timelocked mempool transaction is still valid but becomes valid at a different block, the cached `LockPoints` will be incorrect. This PR fixes the bug, adds a test, and adds an assertion at the end of `removeForReorg()` to check that all mempool entries' lockpoints are valid. You can reproduce the bug by running the test added in the [test] commit on the code before the [bugfix] commit. ACKs for top commit: jnewbery: ACK b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b vasild: ACK b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b mzumsande: Code Review ACK b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b hebasto: ACK b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b MarcoFalke: re-ACK b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b 🏁 Tree-SHA512: 16b59f6ff8140d0229079ca1c6b04f2f4a00a2e49931275150e4f3fe5ac4ec109698b083fa6b223ba9511f328271cc1ab081263669d5da020af7fee83c13e401
2021-12-30scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: * 2020: fa0074e2d82928016a43ca408717154a1c70a4db * 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-12-08Mempool Update Cut-Through OptimizationJeremy Rubin
Often when we're updating mempool entries we update entries that we ultimately end up removing the updated entries shortly thereafter. This patch makes it so that we filter for such entries a bit earlier in processing, which yields a mild improvement for these cases, and is negligible overhead otherwise.
2021-12-06[bugfix] update lockpoints correctly during reorgglozow
During a reorg, we re-check timelocks on all mempool entries using CheckSequenceLocks(useExistingLockPoints=false) and remove any now-invalid entries. CheckSequenceLocks() also mutates the LockPoints passed in, and we update valid entries' LockPoints using update_lock_points. Thus, update_lock_points(lp) needs to be called right after CheckSequenceLocks(lp), otherwise we lose the data in lp. commit bedf246 introduced a bug by separating those two loops.
2021-12-06MOVEONLY: update_lock_points to txmempool.hglozow
2021-12-02clean up txmempool includesglozow
2021-12-02change TestLockPointValidity to take a const referenceglozow
The lockpoints are not changed in this function. There is no reason to pass a pointer.
2021-11-30Break validation <-> txmempool circular dependencyglozow
No behavior change. Parameterize removeForReorg using a CChain and callable that encapsulates validation logic. The mempool shouldn't need to know a bunch of details about coinbase maturity and lock finality. Instead, just pass in a callable function that says true/false. Breaks circular dependency by removing txmempool's dependency on validation.
2021-11-30[mempool] always assert coin spentglozow
This is an extremely cheap function (just checks that the coin CTxOut isn't null) that doesn't need to be gated on check_ratio.
2021-11-30[refactor] put finality and maturity checking into a lambdaglozow
No behavior change.
2021-11-30[mempool] only update lockpoints for non-removed entriesglozow
Each entry's lockpoints are independent of one another, so there isn't any reason to update lockpoints for entries that will be removed. Separating the loops also allows us to move validation logic out and leave lockpoints in txmempool.
2021-11-30MOVEONLY: TestLockPointValidity to txmempoolglozow
2021-11-03Merge bitcoin/bitcoin#23211: refactor: move `update_*` structs from ↵MarcoFalke
txmempool.h to .cpp file 65aaf9495d19ea3fb875228a7e14aab6c1f2986d refactor: move `update_*` structs from txmempool.h to .cpp file (Sebastian Falbesoner) 9947ce62626c05bd186ae8a4864aa382f673ec1a refactor: use const reference for parents in `CTxMemPool::UpdateAncestorsOf` (Sebastian Falbesoner) Pull request description: These helpers are exclusively used in txmempool.cpp, hence they should also be moved there. The PR also contains a commit which fixes const-correctness for parents in `CTxMemPool::UpdateAncestorsOf` and declares them as reference to avoid a copy. ACKs for top commit: promag: Code review ACK 65aaf9495d19ea3fb875228a7e14aab6c1f2986d. Verified move-only commit locally. Tree-SHA512: 7ce29f3ba0e68b5355001f27725b00f6d54cc993015356eb40b61b8cdd17db49b980f4c3d798c8e0c940d245dc3a72c474bb9ff3c0ee971ead450786076812c2
2021-11-02doc: Remove fee delta TODO from txmempool.cppMarcoFalke
2021-10-25Merge bitcoin/bitcoin#23157: txmempool -/-> validation 1/2: improve ↵MarcoFalke
performance of check() and remove dependency on validation 082c5bf099c64e3d27abe9b68a71ce500b693e7e [refactor] pass coinsview and height to check() (glozow) ed6115f1eae0eb4669601106a9aaff078a2f3a74 [mempool] simplify some check() logic (glozow) 9e8d7ad5d9cc4b013826daead9cee09aad539401 [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow) 09d18916afb0ecae90700d4befd9d5dc52767970 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow) e8639ec26aaf4de3fae280963434bf1cf2017b6f [mempool] remove now-unnecessary code (glozow) 54c6f3c1da01090aee9691a2c2bee0984a054ce8 [mempool] speed up check() by using coins cache and iterating in topo order (glozow) 30e240f65e69c6dffcd033afc63895345bd51f53 [bench] Benchmark CTxMemPool::check() (glozow) cb1407196fba648aa75504e3ab3d46aa0181563a [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow) Pull request description: Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`. This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children. ACKs for top commit: jnewbery: reACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e GeneFerneau: tACK [082c5bf](https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e) rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e theStack: Code-review ACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
2021-10-22remove unused CTxMemPool::info(const uint256& txid)MarcoFalke
2021-10-21[mempool] delete exists(uint256) functionglozow
Allowing callers to pass in a uint256 (which could be txid or wtxid) but then always assuming that it's a txid is a footgunny interface.
2021-10-07refactor: move `update_*` structs from txmempool.h to .cpp fileSebastian Falbesoner
These helpers are exclusively used in txmempool.cpp, hence they should also be moved there. Can be reviewed with "--color-moved=dimmed-zebra".
2021-10-07refactor: use const reference for parents in `CTxMemPool::UpdateAncestorsOf`Sebastian Falbesoner
2021-10-04[refactor] pass coinsview and height to check()glozow
Removes check's dependency on validation.h
2021-10-04[mempool] simplify some check() logicglozow
No transaction in the mempool should ever be a coinbase. Since mempoolDuplicate's backend is the chainstate coins view, it should always contain the coins available.
2021-10-04[validation/mempool] use Spend/AddCoin instead of UpdateCoinsglozow
UpdateCoins is an unnecessary dependency on validation. All we need to do is add and remove coins to check inputs. We don't need the extra logic for checking coinbases and handling TxUndos. Also remove the wrapper function in validation.h which constructs a throwaway TxUndo object before calling UpdateCoins because it is now unused.
2021-10-04MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoinsglozow
2021-10-04[mempool] remove now-unnecessary codeglozow
Remove variables used for keeping track of mempool transactions for which we haven't processed the parents yet. Since we're iterating in topological order now, they're always unused.
2021-10-04[mempool] speed up check() by using coins cache and iterating in topo orderglozow
No behavior changes. Before, we're always adding transactions to the "check later" queue if they have any parents in the mempool. But there's no reason to do this if all of its inputs are already available from mempoolDuplicate. Instead, check for inputs, and only mark fDependsWait=true if the parents haven't been processed yet. Reduce the amount of "check later" transactions by looking at ancestors before descendants. Do this by iterating through them in ascending order by ancestor count. This works because a child will always have more in-mempool ancestors than its parent. We should never have any entries in the "check later" queue after this commit.
2021-09-21Use C++11 member initializer in CTxMemPoolEntryMarcoFalke
This removes a bunch of boilerplate, makes the code easier to read. Also, C++11 member initialization avoids accidental uninitialized members. Can be reviewed with the git option "--word-diff-regex=." or with "git difftool --tool=meld".
2021-09-20Fix BlockAssembler::AddToBlock, CTxMemPool::PrioritiseTransaction loggingJon Atack
2021-09-20Merge bitcoin/bitcoin#12677: RPC: Add ancestor{count,size,fees} to ↵W. J. van der Laan
listunspent output 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 doc/release-notes: Add new listunspent fields (Luke Dashjr) 0be2f17ef5649c2d77efbbbdd9222332b2ebf0d2 QA: Add tests for listunspent ancestor{count,size,fees} to mempool_packages (Luke Dashjr) 6966e80f453c46d5d0a923118205f19ac2f4e336 RPC: Add ancestor{count,size,fees} to listunspent output (Luke Dashjr) 3f77dfdaf0f0bfe0c4662a616d6943f31bdd5bf4 Expose ancestorsize and ancestorfees via getTransactionAncestry (Luke Dashjr) Pull request description: Requested by a user ACKs for top commit: prayank23: reACK https://github.com/bitcoin/bitcoin/commit/6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 fjahr: Code review re-ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 kiminuo: ACK [6cb60f3](https://github.com/bitcoin/bitcoin/commit/6cb60f3e6d652ffa4cf570426a7cf1f690d15c45) achow101: Code Review ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 naumenkogs: ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 darosior: utACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 Tree-SHA512: 5d16e5799558691e5853ab7ea2cc85514cb45da3ce69134d855c71845beef32ec6af5ab28d4462683e9800c8ea126f162773a9d3d5660edac08fd8edbfeda173
2021-08-05[mempool] check ancestor/descendant limits for packagesglozow
When calculating ancestor/descendant counts for transactions in the package, as a heuristic, count every transaction in the package as an ancestor and descendant of every other transaction in the package. This may overestimate, but will not underestimate, the ancestor/descendant counts. This shortcut still produces an accurate count for packages of 1 parent + 1 child.
2021-08-05[refactor] pass size/count instead of entry to CalculateAncestorsAndCheckLimitsglozow
This does not change existing behavior. The ancestor/descendant limits are inclusive of the entries themselves, but CalculateAncestorsAndCheckLimits() does not need access to them.
2021-08-05MOVEONLY: add helper function for calculating ancestors and checking limitsglozow
2021-08-01Expose ancestorsize and ancestorfees via getTransactionAncestryLuke Dashjr
2021-06-10scripted-diff: tree-wide: Remove all review-only assertionsCarl Dong
-BEGIN VERIFY SCRIPT- find_regex='((assert|CHECK_NONFATAL)\(std::addressof|TODO: REVIEW-ONLY)' \ && git grep -l -E "$find_regex" -- . \ | xargs sed -i -E "/${find_regex}/d" -END VERIFY SCRIPT-
2021-06-02[refactor] comment/naming improvementsglozow
2021-05-20[coins/mempool] extend CCoinsViewMemPool to track temporary coinsglozow
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.
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 01:32:18 +0000