WIP: Transaction Pool Layer

MarcoFalke commented at 2:48 pm on July 30, 2018: member

This implements a layer around an immutable tx pool. The layer can be seen as a temporary throw-away shell that provides the same interface as CTxMemPool. Its primary purpose right now is to be passed into ATMP while testing acceptance of several (potentially depending) transaction and then to be discarded.

One use case could be to determine if smart contracts that are set up with multiple txs would be accepted by current consensus and policy rules.

In the future it could be extended to support recursive wrapping of layers or a way to commit changes that happened in the layer to the underlying pool or layer. Furthermore, it could be extended to be revivable after changes to the underlying layer happened. (As opposed to be single-use)

MarcoFalke added the label Validation on Jul 30, 2018

DrahtBot commented at 5:42 pm on July 30, 2018: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15681 ([mempool] Allow one extra single-ancestor transaction per package by TheBlueMatt)
#15192 (Add missing cs_main locks in ThreadImport(…)/Shutdown(…)/gettxoutsetinfo(…)/InitScriptExecutionCache(). Add missing locking annotations. by practicalswift)
#15169 (Parallelize CheckInputs() in AcceptToMemoryPool() by sdaftuar)
#10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

MarcoFalke force-pushed on Jul 30, 2018

in src/policy/rbf.h:26 in fa7a9de343 outdated

22@@ -23,6 +23,6 @@ bool SignalsOptInRBF(const CTransaction &tx);
23 // according to BIP 125
24 // This involves checking sequence numbers of the transaction, as well
25 // as the sequence numbers of all in-mempool ancestors.
26-RBFTransactionState IsRBFOptIn(const CTransaction &tx, CTxMemPool &pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
27+RBFTransactionState IsRBFOptIn(const CTransaction &tx,const CTxMemPool &pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs);

promag commented at 2:05 pm on July 31, 2018:

Commit “Stacked Transaction Pool”

nit, add space after ,.

in src/txmempool.cpp:353 in fa7a9de343 outdated

351+void CTxMemPool::UpdateForRemoveFromMempool(const setEntries& entriesToRemove, bool updateDescendants)
352+{
353+    return ::UpdateForRemoveFromMempool(*this, entriesToRemove, updateDescendants);
354+}
355 
356+void CTxMemPool::Layer::  UpdateForRemoveFromMempool(const setEntries&entriesToRemove, bool updateDescendants)

promag commented at 2:06 pm on July 31, 2018:

Commit “Stacked Transaction Pool”

nit, remove extra spaces.

promag commented at 2:08 pm on July 31, 2018: member

Last commit is overwhelming. Could prefix with wip: until dependencies are merged?

MarcoFalke renamed this:
~~Stacked Transaction Pool Layer~~
Transaction Pool Layer
on Aug 4, 2018

MarcoFalke force-pushed on Aug 4, 2018

DrahtBot added the label Needs rebase on Aug 25, 2018

MarcoFalke force-pushed on Aug 29, 2018

DrahtBot removed the label Needs rebase on Aug 29, 2018

MarcoFalke force-pushed on Aug 29, 2018

DrahtBot added the label Needs rebase on Sep 4, 2018

MarcoFalke force-pushed on Sep 7, 2018

DrahtBot removed the label Needs rebase on Sep 7, 2018

MarcoFalke force-pushed on Sep 9, 2018

laanwj referenced this in commit e0a4f9de7f on Sep 10, 2018

DrahtBot added the label Needs rebase on Sep 10, 2018

MarcoFalke force-pushed on Sep 10, 2018

DrahtBot removed the label Needs rebase on Sep 10, 2018

in src/txmempool.cpp:1011 in 7bbdcd3a04 outdated

1013+boost::optional<CTxMemPool::Layer::txiter> CTxMemPool::Layer::GetIter(const uint256& txid) const
1014 {
1015-    CTxMemPool::setEntries ret;
1016+    const auto& it_cache = m_cache_added.find(txid);
1017+    if (it_cache != m_cache_added.end()) {
1018+        CTxMemPool::Layer::txiter{it_cache};

practicalswift commented at 4:10 pm on September 14, 2018:

Ownerless expression?

practicalswift commented at 10:06 am on September 17, 2018: contributor

This PR fails to compile with thread safety analysis enabled when rebased on master.

MarcoFalke commented at 11:01 pm on September 17, 2018: member

@practicalswift I think clang doesn’t understand “recursive” annotations. I am happy to be proven wrong, but I think this just doesn’t compile with annotations for now.

MarcoFalke force-pushed on Sep 19, 2018

in src/txmempool.h:607 in 554c3b8592 outdated

574@@ -575,6 +575,9 @@ class CTxMemPool
575     /** Translate a set of hashes into a set of pool iterators to avoid repeated lookups */
576     setEntries GetIterSet(const std::set<uint256>& hashes) const EXCLUSIVE_LOCKS_REQUIRED(cs);
577 
578+    /** Dereference the txiter */
579+    static const CTxMemPoolEntry& GetEntry(txiter it) { return *it; };

practicalswift commented at 8:11 am on September 23, 2018:

02018-09-22 21:49:48 cpplint(pr=13804): src/txmempool.h:579:  You don't need a ; after a }  [readability/braces] [4]

in src/txmempool.h:757 in 554c3b8592 outdated

724+         */
725+        CCriticalSection& cs;
726+
727+        struct txiter_nested {
728+            CTxMemPool::txiter _i;
729+            explicit txiter_nested(CTxMemPool::txiter val) : _i(val){};

practicalswift commented at 8:11 am on September 23, 2018:

02018-09-22 21:49:48 cpplint(pr=13804): src/txmempool.h:729:  You don't need a ; after a }  [readability/braces] [4]

in src/txmempool.h:758 in 554c3b8592 outdated

725+        CCriticalSection& cs;
726+
727+        struct txiter_nested {
728+            CTxMemPool::txiter _i;
729+            explicit txiter_nested(CTxMemPool::txiter val) : _i(val){};
730+            txiter_nested() : _i(){};

practicalswift commented at 8:12 am on September 23, 2018:

02018-09-22 21:49:48 cpplint(pr=13804): src/txmempool.h:730:  You don't need a ; after a }  [readability/braces] [4]

in src/txmempool.h:759 in 554c3b8592 outdated

754+        using txlinksMap = std::map<CTxMemPool::Layer::txiter, CTxMemPool::Layer::TxLinks, CompareIteratorByHash>;
755+        /**
756+         * Construct a Layer based on an existing tx pool.
757+         * The caller must acquire the lock for the whole life-time of this layer.
758+         */
759+        Layer(const CTxMemPool& p) EXCLUSIVE_LOCKS_REQUIRED(m_tx_pool.cs)

practicalswift commented at 8:12 am on September 23, 2018:

02018-09-22 21:49:48 cpplint(pr=13804): src/txmempool.h:759:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [5]

in src/txmempool.h:808 in 554c3b8592 outdated

803+        void UpdateChildrenForRemoval(txiter it);
804+        const setEntries& GetMemPoolChildren(txiter entry) const;
805+        void removeUnchecked(txiter it, MemPoolRemovalReason reason);
806+
807+    private:
808+

practicalswift commented at 8:13 am on September 23, 2018:

02018-09-22 21:49:48 cpplint(pr=13804): src/txmempool.h:808:  Do not leave a blank line after "private:"  [whitespace/blank_line] [3]

in src/txmempool.cpp:468 in 554c3b8592 outdated

469-    vTxHashes.emplace_back(tx.GetWitnessHash(), newit);
470+    vTxHashes.emplace_back(entry.GetTx().GetWitnessHash(), newit);
471     newit->vTxHashesIdx = vTxHashes.size() - 1;
472 }
473 
474+void CTxMemPool::Layer::addUnchecked(const CTxMemPoolEntry& entry, const CTxMemPool::Layer::setEntries& setAncestors, bool /* validFeeEstimate */ ignore)

practicalswift commented at 5:13 am on September 26, 2018:

Use an unnamed parameter instead of ignore.

in src/txmempool.cpp:474 in 554c3b8592 outdated

475+{
476+    ::addUnchecked(*this, entry, setAncestors);
477+}
478+
479+template <typename P>
480+static void removeUnchecked(P& pool, typename P::txiter it, MemPoolRemovalReason reason)

practicalswift commented at 5:13 am on September 26, 2018:

Make MemPoolRemovalReason an unnamed parameter.

in src/txmempool.cpp:898 in 554c3b8592 outdated

903+    auto it = m_cache_added.find(hash);
904+    if (it == m_cache_added.end())
905+        return nullptr;
906+    else
907+        return it->GetSharedTx();
908+    if (m_cache_removed.find(hash) != m_cache_removed.end()) return nullptr;

practicalswift commented at 5:14 am on September 26, 2018:

This code will never get executed.

in src/txmempool.cpp:1139 in 554c3b8592 outdated

1135@@ -937,14 +1136,31 @@ int CTxMemPool::Expire(int64_t time) {
1136     RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY);
1137     return stage.size();
1138 }
1139+int CTxMemPool::Layer::Expire(int64_t time)

practicalswift commented at 5:15 am on September 26, 2018:

Make time an unnamed parameter.

in src/txmempool.cpp:1319 in 554c3b8592 outdated

1315@@ -1058,6 +1316,13 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
1316     }
1317 }
1318 
1319+void CTxMemPool::Layer::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining)

practicalswift commented at 5:16 am on September 26, 2018:

Make sizelimit and pvNoSpendsRemaining unnamed parameters.

MarcoFalke force-pushed on Sep 26, 2018

MarcoFalke force-pushed on Oct 27, 2018

MarcoFalke renamed this:
~~Transaction Pool Layer~~
WIP: Transaction Pool Layer
on Oct 28, 2018

MarcoFalke force-pushed on Oct 28, 2018

in src/txmempool.cpp:1000 in f1d3f1b93a outdated

994+{
995+    auto it_add = m_map_next_tx_added.find(prevout);
996+    if (it_add != m_map_next_tx_added.end()) return it_add->second;
997+    if (m_map_next_tx_removed.find(prevout) != m_map_next_tx_removed.end()) return nullptr;
998+
999+    return m_tx_pool.GetConflictTx(prevout);

practicalswift commented at 12:25 pm on December 8, 2018:

Calling GetConflictTx requires holding the mutex m_tx_pool.cs

in src/txmempool.cpp:1021 in f1d3f1b93a outdated

1017+    if (m_cache_removed.find(txid) != m_cache_removed.end()) {
1018+        return boost::optional<CTxMemPool::Layer::txiter>{};
1019+    }
1020+
1021+    // Nothing found in the cache, fall back to the inner layer:
1022+    const auto it_inner = m_tx_pool.GetIter(txid);

practicalswift commented at 12:26 pm on December 8, 2018:

m_tx_pool.cs must be held when calling GetIter

in src/txmempool.cpp:1222 in f1d3f1b93a outdated

1210+{
1211+    const auto it = m_cache_map_links.find(entry);
1212+    if (it != m_cache_map_links.end()) return it->second.parents;
1213+
1214+    CTxMemPool::Layer::setEntries ret;
1215+    for (const auto& parent : m_tx_pool.GetMemPoolParents(boost::get<CTxMemPool::txiter>(entry))) {

practicalswift commented at 12:26 pm on December 8, 2018:

m_tx_pool.cs must be held here

practicalswift commented at 12:28 pm on December 8, 2018: contributor

This PR doesn’t compile due to missing locks

DrahtBot added the label Needs rebase on Dec 13, 2018

MarcoFalke force-pushed on Dec 22, 2018

DrahtBot removed the label Needs rebase on Dec 24, 2018

DrahtBot added the label Needs rebase on Jan 15, 2019

MarcoFalke force-pushed on Mar 7, 2019

DrahtBot removed the label Needs rebase on Mar 7, 2019

MarcoFalke force-pushed on Mar 8, 2019

DrahtBot added the label Needs rebase on Mar 21, 2019

MarcoFalke force-pushed on Jul 10, 2019

DrahtBot removed the label Needs rebase on Jul 10, 2019

MarcoFalke force-pushed on Jul 11, 2019

txpool: Avoid mapTx.iterator_to lookup in CalculateMemPoolAncestors b33a0fb7f9

tx pool: Make pool a template parameter of CoinsViewMemPool a6bf4f0588

tx pool: Move ATMP internal logic into static functions for template-reusability 980353a186

txmempool: Use auto and GetEntry to hide txiter type 19c277055c

MarcoFalke force-pushed on Jul 13, 2019

Transaction Pool Layer f57817fca0

rpc: Use TxPoolLayer in testmempoolaccept 421de16e9c

MarcoFalke force-pushed on Jul 13, 2019

MarcoFalke closed this on Jul 16, 2019

MarcoFalke deleted the branch on Jul 16, 2019

Munkybooty referenced this in commit 208626a527 on Jul 7, 2021

5tefan referenced this in commit 63fa38955b on Aug 10, 2021

5tefan referenced this in commit 7b6837b117 on Aug 11, 2021

5tefan referenced this in commit f719cafdcd on Aug 12, 2021

5tefan referenced this in commit 495cfa67f2 on Aug 12, 2021

5tefan referenced this in commit db302f912d on Aug 12, 2021

DrahtBot locked this on Dec 16, 2021

WIP: Transaction Pool Layer #13804

Conflicts