Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC5471411dba6d: Migrate the mempool to hold shared pointers

Summary

The mempool actually stores CTxMemPoolEntry objects, which are wrappers
around transactions with added/cached statistics. Since there is a
single mempool this is not a problem and all the algorithms are using
iterators and/or references to manipulate the objects that remain owned
by the mempool.

With pre-consensus we will need to move these entries to other data
structures: the conflicting txs will be voted on and need to be stored,
and we want to build a tree of the finalized txs that will be used to
generate the block template.

In order to not lose and having to recompute the cached data (especially
the fees) we need to be able to move the entries from one container to
another. Using a shared pointer is a step in this direction. In the end
we will probably use a RCUPtr but this require additional refactors,
as it makes the entry non copyable. This first step is the purpose of
this diff.

Depends on D13439 and D13440.

Test Plan

With debug:

ninja all check-all

ninja check-extended

Run the ASAN, UBSAN and TSAN builds.

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Mar 23 2023, 16:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 23 2023, 16:24

Harbormaster completed remote builds in B22624: Diff 38801.Mar 23 2023, 16:39

Improve comment on CompareIteratorById

Harbormaster completed remote builds in B22649: Diff 38831.Mar 24 2023, 11:02

Fix a use after move issue

Fabien added inline comments.Mar 28 2023, 08:28

src/txmempool.h
103 ↗	(On Diff #38994)	This needs to be refactored, or updated to not hold shared pointers. It creates a circular reference which causes a memory leak upon entry destruction, because the reference counter will never reach 0.

Harbormaster completed remote builds in B22788: Diff 38994.Mar 28 2023, 08:41

Rebase

Harbormaster completed remote builds in B26168: Diff 43924.Jan 5 2024, 13:47

Rebase

Fabien published this revision for review.Jan 12 2024, 15:21

Fabien added a child revision: D15147: Avoid a copy when inserting a mempool entry.Jan 12 2024, 15:21

Harbormaster completed remote builds in B26290: Diff 44096.Jan 12 2024, 15:35

PiRK accepted this revision.Jan 12 2024, 16:14

This revision is now accepted and ready to land.Jan 12 2024, 16:14

PiRK edited the summary of this revision. (Show Details)Jan 12 2024, 16:15

Closed by commit rABC5471411dba6d: Migrate the mempool to hold shared pointers (authored by Fabien). · Explain WhyJan 15 2024, 13:18

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABC5471411dba6d: Migrate the mempool to hold shared pointers.

Migrate the mempool to hold shared pointers
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 44173

src/avalanche/test/processor_tests.cpp

src/blockencodings.cpp

src/net_processing.cpp

src/node/interfaces.cpp

src/node/miner.h

src/node/miner.cpp

src/rpc/blockchain.cpp

src/test/blockencodings_tests.cpp

src/test/mempool_tests.cpp

src/txmempool.h

src/txmempool.cpp

src/validation.cpp

Migrate the mempool to hold shared pointersClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 44173

src/avalanche/test/processor_tests.cpp

src/blockencodings.cpp

src/net_processing.cpp

src/node/interfaces.cpp

src/node/miner.h

src/node/miner.cpp

src/rpc/blockchain.cpp

src/test/blockencodings_tests.cpp

src/test/mempool_tests.cpp

src/txmempool.h

src/txmempool.cpp

src/validation.cpp

Migrate the mempool to hold shared pointers
ClosedPublic
Actions

Revision Contents
Changeset List