aboutsummaryrefslogtreecommitdiff
path: root/src/txmempool.h
AgeCommit message (Collapse)Author
2023-11-16Merge bitcoin/bitcoin#28605: Fix typosfanquake
43de4d3630274e1287179c86896ed4c2d8b9eff4 doc: fix typos (Sjors Provoost) Pull request description: This PR fixes typos found by lint-spelling.py using codespell 2.2.6. Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now. ACKs for top commit: pablomartin4btc: re ACK 43de4d3630274e1287179c86896ed4c2d8b9eff4 Tree-SHA512: c032fe86cb49c924a468385653b31f309a9db68c478d70335bba3e65a1ff3826abe80284fe00a090ab5a509e1edbf17e476f6922fb15d055e50f1103dad2ccb0
2023-11-13Merge bitcoin/bitcoin#28207: mempool: Persist with XORAndrew Chow
fa6b053b5c964fb35935fa994cb782c0731a56f8 mempool: persist with XOR (MarcoFalke) Pull request description: Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it. While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart. This may cause fee estimates to be off, or may cause block relay to be slower. Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat file when writing or reading it. Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat file. Any program that intentionally wants to mess with the dat file can still trivially do so. ACKs for top commit: achow101: re-ACK fa6b053b5c964fb35935fa994cb782c0731a56f8 glozow: reACK fa6b053b5c964fb35935fa994cb782c0731a56f8 ismaelsadeeq: ACK fa6b053b5c964fb35935fa994cb782c0731a56f8 Tree-SHA512: ded2ce3d81bc944b828263534e3178a1e45a914fe8e024f4a14c6561a73e301820944ecc75dd704b3d4221a7a3a5c0597ccab79546250c1197609ee981fe324e
2023-11-10scripted-diff: rename vTxHashes to txns_randomizedTheCharlatan
-BEGIN VERIFY SCRIPT- git grep -l "vTxHashesIdx" src | xargs sed -i "s/vTxHashesIdx/idx_randomized/g" git grep -l "vTxHashes" src | xargs sed -i "s/vTxHashes/txns_randomized/g" -END VERIFY SCRIPT-
2023-11-10[refactor] rewrite vTxHashes as a vector of CTransactionRefglozow
vTxHashes exposes a complex mapTx iterator type that its external users don't need. Directly populate it with CTransactionRef instead.
2023-11-10[refactor] Add helper for retrieving mempool entryTheCharlatan
In places where the iterator is only needed for accessing the actual entry, it should not be required to first retrieve the iterator.
2023-11-10[refactor] Add helper for iterating through mempool entriesstickies-v
Instead of reaching into the mapTx data structure, use a helper method that provides the required vector of CTxMemPoolEntry pointers.
2023-11-09mempool: persist with XORMarcoFalke
2023-11-07doc: fix typosSjors Provoost
As found by lint-spelling.py using codespell 2.2.6.
2023-09-23Merge bitcoin/bitcoin#28385: [refactor] rewrite ↵fanquake
DisconnectedBlockTransactions to not use boost 4313c77400eb8eaa8586db39a7e29a861772ea80 make DisconnectedBlockTransactions responsible for its own memory management (glozow) cf5f1faa037e9a40a5029cc7dd4ee61454b62466 MOVEONLY: DisconnectedBlockTransactions to its own file (glozow) 2765d6f3434c101fe2d46e9313e540aa680fbd77 rewrite DisconnectedBlockTransactions as a list + map (glozow) 79ce9f0aa46de8ff742be83fd6f68eab40e073ec add std::list to memusage (glozow) 59a35a7398f5bcb3e3805d1e4f363e4c2fb336b3 [bench] DisconnectedBlockTransactions (glozow) 925bb723ca71aa76380b769d8926c7c2ad9bbb7b [refactor] batch-add transactions to DisconnectedBlockTransactions (glozow) Pull request description: Motivation - I think it's preferable to use stdlib data structures instead of depending on boost if we can achieve the same thing. - Also see #28335 for further context/motivation. This PR simplifies that one. Things done in this PR: - Add a bench for `DisconnectedBlockTransactions` where we reorg and the new chain has {100%, 90%, 10%} of the same transactions. AFAIU in practice, it's usually close to 100%. - Rewrite `DisconnectedBlockTransactions` as a `std::list` + `unordered_map` instead of a boost multi index container. - On my machine, the bench suggests the performance is very similar. - Move `DisconnectedBlockTransactions` from txmempool.h to its own kernel/disconnected_transactions.h. This struct isn't used by txmempool and doesn't have much to do with txmempool. My guess is that it's been living there for convenience since the boost includes are there. ACKs for top commit: ismaelsadeeq: Tested ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 stickies-v: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 TheCharlatan: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 Tree-SHA512: 273c80866bf3acd39b2a039dc082b7719d2d82e0940e1eb6c402f1c0992e997256722b85c7e310c9811238a770cfbdeb122ea4babbc23835d17128f214a1ef9e
2023-09-20Bugfix: Pass correct virtual size to CheckPackageLimitsLuke Dashjr
2023-09-14Remove MemPoolAccept::m_limits, only have local copies for carveoutsGreg Sanders
2023-09-13Merge bitcoin/bitcoin#28251: validation: fix coins disappearing mid-package ↵fanquake
evaluation 32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 [test] mempool coins disappearing mid-package evaluation (glozow) a67f460c3fd1c7eb8070623666d887eefccff0d6 [refactor] split setup in mempool_limit test (glozow) d08696120e3647b4c2cd0ae8d6e57dea12418b7c [test framework] add ability to spend only confirmed utxos (glozow) 3ea71feb11c261f002ed918f91f3434fd8a23589 [validation] don't LimitMempoolSize in any subpackage submissions (glozow) d227b7234cd4cfd7c593ffcf8e2f24573d1ebea5 [validation] return correct result when already-in-mempool tx gets evicted (glozow) 9698b81828ff98820fa49c83ca364063233374c6 [refactor] back-fill results in AcceptPackage (glozow) 8ad7ad33929ee846a55a43c55732be0cb8973060 [validation] make PackageMempoolAcceptResult members mutable (glozow) 03b87c11ca0705e1d6147b90da33ce555f9f41c8 [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow) 3f01a3dab1c4ee37fd4093b6a0a3b622f53e231d [CCoinsViewMemPool] track non-base coins and allow Reset (glozow) 7d7f7a1189432b1b6245ba25df572229870567cb [policy] check for duplicate txids in package (glozow) Pull request description: While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore. Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out. Pointed out by instagibbs in https://github.com/bitcoin/bitcoin/commit/faeed687e5cde5e32750d93818dd1d4add837f24 on top of the v3 PR. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/28251/commits/32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90
2023-09-13[CCoinsViewMemPool] track non-base coins and allow Resetglozow
Temporary coins should not be available in separate subpackage submissions. Any mempool coins that are cached in m_view should be removed whenever mempool contents change, as they may be spent or no longer exist.
2023-09-13MOVEONLY: DisconnectedBlockTransactions to its own fileglozow
This struct is only used in validation + tests and has very little to do with txmempool.
2023-09-13rewrite DisconnectedBlockTransactions as a list + mapglozow
And encapsulate underlying data structures to avoid misuse. It's better to use stdlib instead of boost when we can achieve the same thing. Behavior change: the number returned by DynamicMemoryUsage for the same transactions is higher (due to change in usage or more accurate accounting), which effectively decreases the maximum amount of transactions kept for resubmission in a reorg. Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2023-09-07[refactor] batch-add transactions to DisconnectedBlockTransactionsglozow
No behavior change. In a future commit, we can optimize by reserving vtx.size().
2023-08-17move-only: Create src/kernel/mempool_removal_reason.hMarcoFalke
This is needed for a future commit. Can be reviewed with: --color-moved=dimmed-zebra
2023-08-17Remove unused includes from txmempool.hMarcoFalke
... and move them to where they are really needed. This was found by IWYU: txmempool.h should remove these lines: - #include <random.h> // lines 29-29 - class CBlockIndex; // lines 43-43 - class Chainstate; // lines 45-45 Also, move the stdlib section to the right place. Can be reviewed with: --color-moved=dimmed-zebra
2023-08-17Merge bitcoin/bitcoin#27675: p2p: Drop m_recently_announced_invs bloom filterfanquake
fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 mempool_entry: improve struct packing (Anthony Towns) 1a118062fbc4ec8f645f4ec4298d869a869c3344 net_processing: Clean up INVENTORY_BROADCAST_MAX constants (Anthony Towns) 6fa49937e488d0924044786c76b42324b659f351 test: Check tx from disconnected block is immediately requestable (glozow) e4ffabbffacc4b890d393aafcc8286916ef887d8 net_processing: don't add txids to m_tx_inventory_known_filter (Anthony Towns) 6ec1809d33bfc42b80cb6f35625dccd56be8d507 net_processing: drop m_recently_announced_invs bloom filter (Anthony Towns) a70beafdb22564043dc24fc98133fdadbaf77d8a validation: when adding txs due to a block reorg, allow immediate relay (Anthony Towns) 1e9684f39fba909b3501e9402d5b61f4bf744ff2 mempool_entry: add mempool entry sequence number (Anthony Towns) Pull request description: This PR replaces the `m_recently_announced_invs` bloom filter with a simple sequence number tracking the mempool state when we last considered sending an INV message to a node. This saves 33kB per peer (or more if we raise the rate at which we relay transactions over the network, in which case we would need to increase the size of the bloom filter proportionally). The philosophy here (compare with #18861 and #19109) is that we consider the rate limiting on INV messages to only be about saving bandwidth and not protecting privacy, and therefore after you receive an INV message, it's immediately fair game to request any transaction that was in the mempool at the time the INV message was sent. We likewise consider the BIP 133 feefilter and BIP 37 bloom filters to be bandwidth optimisations here, and treat transactions as requestable if they would have been announced without those filters. Given that philosophy, tracking the timestamp of the last INV message and comparing that against the mempool entry time allows removal of each of `m_recently_announced_invs`, `m_last_mempool_req` and `UNCONDITIONAL_RELAY_DELAY` and associated logic. ACKs for top commit: naumenkogs: ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 amitiuttarwar: review ACK fb02ba3c5f5 glozow: reACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 Tree-SHA512: cbba5ee04c86df26b6057f3654c00a2b45ec94d354f4f157a769cecdaa0b509edaac02b3128afba39b023e82473fc5e28c915a787f84457ffe66638c6ac9c2d4
2023-08-07doc: Clarify the getmempoolinfo.loaded RPC field documentationMarcoFalke
Also, clarify the LoadMempool doxygen.
2023-08-03net_processing: drop m_recently_announced_invs bloom filterAnthony Towns
Rather than using a bloom filter to track announced invs, simply allow a peer to request any tx that entered the mempool prior to the last INV message we sent them. This also obsoletes the UNCONDITIONAL_RELAY_DELAY.
2023-07-22refactor: Make more transaction size variables signedHennadii Stepanov
This change gets rid of `static_cast`s and compiler warnings.
2023-06-12Use `int32_t` type for most transaction size/weight valuesHennadii Stepanov
This change gets rid of a few casts and makes the following commit diff smaller.
2023-06-12Merge bitcoin/bitcoin#27783: Add public Boost headers explicitlyfanquake
2484cacb7a6367b24e924dba0825c843b1dfc1c3 Add public Boost headers explicitly (Hennadii Stepanov) fade2adb5bb4ce9753e7f25da5fb1521f2f503ec test: Avoid `BOOST_ASSERT` macro (Hennadii Stepanov) Pull request description: To check symbols in the code base, run: ``` git grep boost::multi_index::identity git grep boost::multi_index::indexed_by git grep boost::multi_index::tag git grep boost::make_tuple ``` Hoping on the absence of conflicts with top-prio PRs :) ACKs for top commit: MarcoFalke: lgtm ACK 2484cacb7a6367b24e924dba0825c843b1dfc1c3 TheCharlatan: ACK 2484cacb7a6367b24e924dba0825c843b1dfc1c3 Tree-SHA512: d122ab028eee76ee1c4609ed51ec8db0c8c768edcc2ff2c0e420a48e051aa71e99748cdb5d22985ae6d97c808c77c1a27561f0715f77b256f74c1c310b37694c
2023-06-07Merge bitcoin/bitcoin#27501: mempool / rpc: add getprioritisedtransactions, ↵Andrew Chow
delete a mapDeltas entry when delta==0 67b7fecacd0489809690982c89ba2d0acdca938c [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow) c1061acb9d502cdf8c6996c818d9a8a281cbe40c [functional test] prioritisation is not removed during replacement and expiry (glozow) 0e5874f0b06114d9b077e0ff582915e4f83059e6 [functional test] getprioritisedtransactions RPC (glozow) 99f8046829f699ff2eace266aa8cea1d9f7cb65a [rpc] add getprioritisedtransactions (glozow) 9e9ca36c80013749faaf2aa777d52bd07d9d24ec [mempool] add GetPrioritisedTransactions (glozow) Pull request description: Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`. Motivation / Background - `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted. - Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen. - Since #8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart. - Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them. - Related: #4818 and #6464. - There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat. - Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while. Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are. ACKs for top commit: achow101: ACK 67b7fecacd0489809690982c89ba2d0acdca938c theStack: Code-review ACK 67b7fecacd0489809690982c89ba2d0acdca938c instagibbs: code review ACK 67b7fecacd0489809690982c89ba2d0acdca938c ajtowns: ACK 67b7fecacd0489809690982c89ba2d0acdca938c code review only, some nits Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
2023-05-31Add public Boost headers explicitlyHennadii Stepanov
2023-05-10[mempool] add GetPrioritisedTransactionsglozow
2023-03-27[mempool] find connected mempool entries with GatherClusters(…)glozow
We limit GatherClusters’s result to a maximum of 500 transactions as clusters can be made arbitrarily large by third parties. Co-authored-by: Murch <murch@murch.one>
2023-02-01Fix clang-tidy readability-const-return-type violationsMarcoFalke
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-01-04Merge bitcoin/bitcoin#19909: refactor: Remove unused CTxMemPool::clear() helperglozow
fa818e103c0ddb515f29ae9ce8de44931e12e69e txmempool: Remove unused clear() member function (MarcoFalke) Pull request description: Seems odd to have code in Bitcoin Core that is unused. Moreover the function was broken (see https://github.com/bitcoin/bitcoin/pull/24145) and is brittle, as there is nothing that prevents similar bugs from re-appearing. Fix both issues by replacing it with C++11 member initializers. ACKs for top commit: glozow: ACK fa818e103c0ddb515f29ae9ce8de44931e12e69e Tree-SHA512: e79e44cac7d5a84d9ecc8e3f3b0b9a50e1e3ebec358b20ba5dac175ef07d1fbe338a20f83ee80f746f7c726c79e77f8be49e14bca57a41063da8a5302123c3a9
2023-01-03Merge bitcoin/bitcoin#26289: Use util::Result in for calculating mempool ↵Andrew Chow
ancestors 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v) 5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v) f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v) 66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v) Pull request description: Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear. ## Unexpected behaviour This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs. In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit. ## Improvements ### Return value instead of out-parameters This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit. ### Observability There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](https://github.com/bitcoin/bitcoin/blob/69b10212ea5370606c7a5aa500a70c36b4cbb58f/src/node/miner.cpp#L399). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here. ACKs for top commit: achow101: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26289/commits/47c4b1f52ab8d95d7deef83050bad49d1e3e5990 glozow: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 furszy: light code review ACK 47c4b1f5 aureleoules: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
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-13mempool: add AssumeCalculateMemPoolAncestors helper functionstickies-v
There are quite a few places that assume CalculateMemPoolAncestors will return a value without raising an error. This helper function adds logging (and Assume for debug builds) that ensures robustness but increases visibility in case of unexpected failures
2022-12-13mempool: use util::Result for CalculateMemPoolAncestorsstickies-v
Avoid using setAncestors outparameter, simplify function signatures and avoid creating unused dummy strings.
2022-12-13txmempool: Remove unused clear() member functionMarcoFalke
2022-12-12mempool: use util::Result for CalculateAncestorsAndCheckLimitsstickies-v
Avoid using setAncestors outparameter, simplify function signatures and avoid creating unused dummy strings.
2022-11-30refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h`Hennadii Stepanov
2022-11-18Merge bitcoin/bitcoin#17786: refactor: Nuke policy/fees->mempool circular ↵glozow
dependencies c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 refactor: Inline `CTxMemPoolEntry` class's functions (Hennadii Stepanov) 75bbe594e54402ed248ecf2bdfe54e8283d20fff refactor: Move `CTxMemPoolEntry` class to its own module (Hennadii Stepanov) Pull request description: This PR: - gets rid of the `policy/fees` -> `txmempool` -> `policy/fees` circular dependency - is an alternative to #13949, which nukes only one circular dependency ACKs for top commit: ryanofsky: Code review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770. Just include and whitespace changes since last review, and there's a moveonly commit now so it's very easy to review theStack: Code-review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 glozow: utACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770, agree these changes are an improvement. Tree-SHA512: 36ece824e6ed3ab1a1e198b30a906c8ac12de24545f840eb046958a17315ac9260c7de26e11e2fbab7208adc3d74918db7a7e389444130f8810548ca2e81af41
2022-11-16refactor: Move `CTxMemPoolEntry` class to its own moduleHennadii Stepanov
This change nukes the policy/fees->mempool circular dependency. Easy to review using `diff --color-moved=dimmed-zebra`.
2022-11-04log: mempool: log removal reason in validation interfaceJames O'Beirne
Currently the exact reason a transaction is removed from the mempool isn't logged. It is sometimes detectable from context, but adding the `reason` to the validation interface logs (where it is already passed) seems like an easy way to disambiguate. For example, in the case of mempool expiry, the logs look like this: ``` [validationinterface.cpp:220] [TransactionRemovedFromMempool] [validation] Enqueuing TransactionRemovedFromMempool: txid=<txid> wtxid=<wtxid> [txmempool.cpp:1050] [RemoveUnbroadcastTx] [mempool] Removed <txid> from set of unbroadcast txns before confirmation that txn was sent out [validationinterface.cpp:220] [operator()] [validation] TransactionRemovedFromMempool: txid=<txid> wtxid=<wtxid> [validation.cpp:267] [LimitMempoolSize] [mempool] Expired 1 transactions from the memory pool ``` There is no context-free way to know $txid was evicted on the basis of expiry. This change will make that case (and probably others) clear.
2022-10-09Merge bitcoin/bitcoin#26103: refactor: mempool: use CTxMemPool::Limitsglozow
33b12e5df6aa14344dd084e0c6f2d34158fca383 docs: improve docs where MemPoolLimits is used (stickies-v) 6945853c0bf3b1941bfd18b5ffbd5a81be0e56da test: use NoLimits() in MempoolIndexingTest (stickies-v) 3a86f24a4c1e4e985b1d90eddc135b8dd17049a4 refactor: mempool: use CTxMempool::Limits (stickies-v) b85af25f8770974bae4ef3fae64e75ef6dd2d3c2 refactor: mempool: add MemPoolLimits::NoLimits() (stickies-v) Pull request description: Mempool currently considers 4 limits regarding ancestor and descendant count and size, which get passed around between functions quite a bit. This PR uses `CTxMemPool::Limits` introduced in https://github.com/bitcoin/bitcoin/pull/25290 to simplify those signatures and callsites. The purpose of this PR is to improve readability and maintenance, without behaviour change. As noted in the first commit "refactor: mempool: change MemPoolLimits members to uint", we currently have an underflow issue where a user could pass a negative `-limitancestorsize`, which is eventually cast to an unsigned integer. This behaviour already exists. Because it's orthogonal and to minimize scope, I think this should be fixed in a separate PR. ACKs for top commit: hebasto: ACK 33b12e5df6aa14344dd084e0c6f2d34158fca383, I have reviewed the code and it looks OK, I agree it can be merged. glozow: reACK 33b12e5df6aa14344dd084e0c6f2d34158fca383 Tree-SHA512: 591c6dcee1894f1c3ca28b34a680eeadcf0d40cda92451b4a422c03087b27d682b5e30ba4367abd75a99b5ccb115b7884b0026958d3c7dddab030549db5a4056
2022-10-07docs: fix m_children to be a member of CTxMemPoolEntrystickies-v
2022-10-05docs: improve docs where MemPoolLimits is usedstickies-v
2022-10-05refactor: mempool: use CTxMempool::Limitsstickies-v
Simplifies function signatures by removing repetition of all the ancestor/descendant limits, and increases readability by being more verbose by naming the limits, while still reducing the LoC.
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-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-02Remove ::g_max_datacarrier_bytes globalMacroFake
2022-08-02Remove ::fIsBareMultisigStd globalMacroFake
2022-08-02Remove ::dustRelayFeeMacroFake