diff options
| author | glozow <gloriajzhao@gmail.com> | 2023-11-02 11:09:17 +0000 |
|---|---|---|
| committer | glozow <gloriajzhao@gmail.com> | 2023-11-02 11:12:17 +0000 |
| commit | 023418a140b05b788b45fcb66bdd4832c08db751 (patch) | |
| tree | 08ab1463c9fac758829e1a9c0cc30215b2529ed1 /src/Makefile.am | |
| parent | 023e8c2001956bc217521399c6be902c1ca9138f (diff) | |
| parent | 9b3da70bd06b45482e7211aa95637a72bd115553 (diff) | |
| download | bitcoin-023418a140b05b788b45fcb66bdd4832c08db751.tar.xz | |
Merge bitcoin/bitcoin#28530: tests, bug fix: DisconnectedBlockTransactions rewrite followups
9b3da70bd06b45482e7211aa95637a72bd115553 [test] DisconnectedBlockTransactions::DynamicMemoryUsage (glozow)
b2d04479647af64ad7cf5ebfb6175251b2f6b72e bugfix: correct DisconnectedBlockTransactions memory usage (stickies-v)
f4254e209801d6a790b5f0c251c0b32154a4e3cc assume duplicate transactions are not added to `iters_by_txid` (ismaelsadeeq)
29eb219c1247993378fce06c8f71aab20736c237 move only: move implementation code to disconnected_transactions.cpp (ismaelsadeeq)
81dfeddea70ae5feeaf79062585c2ff9f33c0ca3 refactor: update `MAX_DISCONNECTED_TX_POOL` from kb to bytes (ismaelsadeeq)
Pull request description:
This PR is a follow-up to fix review comments and a bugfix from #28385
The PR
- Updated `DisconnectedBlockTransactions`'s `MAX_DISCONNECTED_TX_POOL` from kb to bytes.
- Moved `DisconnectedBlockTransactions` implementation code to `kernel/disconnected_transactions.cpp`.
- `AddTransactionsFromBlock` now assume duplicate transactions are not passed by asserting after inserting each transaction to `iters_by_txid`.
- Included a Bug fix: In the current master we are underestimating the memory usage of `DisconnectedBlockTransactions`.
* When adding and subtracting `cachedInnerUsage` we call `RecursiveDynamicUsage` with `CTransaction` which invokes this [`RecursiveDynamicUsage(const CTransaction& tx)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L32) version of `RecursiveDynamicUsage`, the output of that call only account for the memory usage of the inputs and outputs of the `CTransaction`, this omits the memory usage of the `CTransaction` object and the control block.
* This PR fixes this bug by calling `RecursiveDynamicUsage` with `CTransactionRef` when adding and subtracting `cachedInnerUsage` which invokes [`RecursiveDynamicUsage(const std::shared_ptr<X>& p)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L67) version of `RecursiveDynamicUsage` the output of the calculation accounts for the` CTransaction` object, the control blocks, inputs and outputs memory usage.
* see [comment ](https://github.com/bitcoin/bitcoin/pull/28385#discussion_r1322948452)
- Added test for DisconnectedBlockTransactions memory limit.
ACKs for top commit:
stickies-v:
ACK 9b3da70bd06b45482e7211aa95637a72bd115553 - nice work!
BrandonOdiwuor:
re ACK 9b3da70bd06b45482e7211aa95637a72bd115553
glozow:
ACK 9b3da70bd06b45482e7211aa95637a72bd115553
Tree-SHA512: 69b9595d09f4d0209038f97081d790cea92ccf63efb94e9e372749979fcbe527f7f17a8e454720cedd12021be0c8e11cf99874625d3dafd9ec602b12dbeb4098
Diffstat (limited to 'src/Makefile.am')
| -rw-r--r-- | src/Makefile.am | 2 |
1 files changed, 2 insertions, 0 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 8508d13b34..99b2184cf2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -404,6 +404,7 @@ libbitcoin_node_a_SOURCES = \ kernel/coinstats.cpp \ kernel/context.cpp \ kernel/cs_main.cpp \ + kernel/disconnected_transactions.cpp \ kernel/mempool_persist.cpp \ kernel/mempool_removal_reason.cpp \ mapport.cpp \ @@ -944,6 +945,7 @@ libbitcoinkernel_la_SOURCES = \ kernel/coinstats.cpp \ kernel/context.cpp \ kernel/cs_main.cpp \ + kernel/disconnected_transactions.cpp \ kernel/mempool_persist.cpp \ kernel/mempool_removal_reason.cpp \ key.cpp \ |