rpc: testmempoolaccept for list of transactions #16398

1. 
   MarcoFalke commented at 3:05 pm on July 16, 2019: member

   This extends the testmempoolaccept rpc to run on a vector of transactions. The idea is to keep the current mempool immutable and only keep track of the differences to the underlying mempool.

   This is based on the transaction pool layer idea from #13804. Comment from there:

   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)

2. txpool: Avoid mapTx.iterator_to lookup in CalculateMemPoolAncestors b33a0fb7f9
3. tx pool: Make pool a template parameter of CoinsViewMemPool a6bf4f0588
4. tx pool: Move ATMP internal logic into static functions for template-reusability 980353a186
5. txmempool: Use auto and GetEntry to hide txiter type 8a9d2ebc37
6. Transaction Pool Layer 510056c81f
7. fixup to tx pool: Make pool a template parameter of CoinsViewMemPool 5c212de0c2
8. rpc: Use TxPoolLayer in testmempoolaccept fcc0ea1497
9. rpc: Accept a list of txs in testmempoolaccept c3fdb9aad7
10. fixup! txpool: Avoid mapTx.iterator_to lookup in CalculateMemPoolAncestors 197fa405b9
11. fanquake added the label RPC/REST/ZMQ on Jul 16, 2019
12. fanquake added the label Mempool on Jul 16, 2019
13. 
    ryanofsky commented at 4:10 pm on July 16, 2019: member

    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.

    I think I basically understand this change, but not the use cases. I would love an ELI5 of the above testmempoolaccept use case. Specifically who might be calling the testmempoolaccept to build what kind of application.

    Also, it would be useful to know if you think other applications of transaction pool layers could be more or equally useful as testmempoolaccept. IIRC these layers could also help implementing dandelion? Basically, I’d love to see some kind of post like #15605 or #14895 that explains what motivated these changes and how they can be built on and used.

14. 
    DrahtBot commented at 6:43 pm on July 16, 2019: 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:

    • #16421 (Conservatively accept RBF bumps bumping one tx at the package limits by TheBlueMatt)
    • #16409 (Remove mempool expiry, treat txs as replaceable instead by MarcoFalke)
    • #16401 (Package relay by sdaftuar)
    • #16400 ([refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts by sdaftuar)
    • #15921 (Tidy up ValidationState interface by jnewbery)
    • #15681 ([mempool] Allow one extra single-ancestor transaction per package by TheBlueMatt)
    • #15606 ([experimental] UTXO snapshots by jamesob)
    • #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.

15. in src/txmempool.cpp:715 in 197fa405b9

    710+        for (const CTxIn& txin : tx.vin) {
    711+            if (exists(txin.prevout.hash)) {
    712+                setParentCheck.insert(*GetIter(txin.prevout.hash));
    713+            }
    714+            const CTransaction* c_tx = GetConflictTx(txin.prevout);
    715+            assert(c_tx = &tx);
    

    ---

    ---

    

    practicalswift commented at 10:22 am on July 20, 2019:

    👀
16. 
    MarcoFalke commented at 6:52 pm on January 3, 2021: member

    This doesn’t compile/pass tests and a better solution is available in #20833, so please review that.
17. MarcoFalke closed this on Jan 3, 2021

    ---

18. MarcoFalke deleted the branch on Jan 3, 2021
19. DrahtBot locked this on Aug 16, 2022

Contributors
MarcoFalke ryanofsky DrahtBot practicalswift

Labels
RPC/REST/ZMQ Mempool