Do not use mempool for GETDATA for tx accepted after the last mempool req. #8080

1. 
   gmaxwell commented at 5:54 am on May 21, 2016: contributor

   The ability to GETDATA a transaction which has not (yet) been relayed is a privacy loss vector.

   The use of the mempool for this was added as part of the mempool p2p message and is only needed to fetch transactions returned by it.

2. jonasschnelli added the label P2P on May 21, 2016
3. jonasschnelli added the label Mempool on May 21, 2016
4. in src/main.cpp: in ef6c31142e outdated

   4502@@ -4503,9 +4503,12 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
   4503                 }
   4504                 if (!pushed && inv.type == MSG_TX) {
   4505                     CTransaction tx;
   4506-                    if (mempool.lookup(inv.hash, tx)) {
   4507-                        pfrom->PushMessage(NetMsgType::TX, tx);
   4508-                        pushed = true;
   4509+                    int64_t txtime;
   4510+                    if (mempool.lookup(inv.hash, tx, txtime)) {
   4511+                        if (txtime <= pfrom->timeLastMempool) {
   

   ---

   ---

   

   jonasschnelli commented at 5:58 am on May 21, 2016:

   wouldn’t if (mempool.lookup(inv.hash, tx, textile) && txtime <= pfrom->timeLastMempool) { also work?

   ---

   

   gmaxwell commented at 6:12 am on May 21, 2016:

   Indeed, current layout is an artifact of my local copy having an else for logging.
5. 
   jonasschnelli commented at 6:01 am on May 21, 2016: contributor

   utACK ef6c31142e52e56af93486986319f5499e72c7df
6. 
   sipa commented at 6:52 am on May 21, 2016: member

   utACK ef6c31142e52e56af93486986319f5499e72c7df
7. 
   gmaxwell commented at 5:34 am on May 23, 2016: contributor

   Changed the variable name and added a comment in response to Petertodd’s comments, also avoided the nested if per jonasschnelli’s comments.
8. 
   petertodd commented at 8:28 pm on May 23, 2016: contributor

   utACK https://github.com/bitcoin/bitcoin/pull/8080/commits/8e9890f4ecbc08676d66479ed473d06036ca82be
9. in src/net.h: in 8e9890f4ec outdated

   413@@ -413,6 +414,8 @@ class CNode
   414     // Used for BIP35 mempool sending, also protected by cs_inventory
   415     bool fSendMempool;
   416 
   417+    // Last time a mempool
   

   ---

   ---

   

   paveljanik commented at 5:22 am on May 24, 2016:

   Please extend the comment here, e.g. Last time of mempool BIP35 request?
10. in src/txmempool.cpp: in 8e9890f4ec outdated

    797@@ -798,6 +798,16 @@ bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
    798     return true;
    799 }
    800 
    801+bool CTxMemPool::lookup(uint256 hash, CTransaction& result, int64_t& time) const
    

    ---

    ---

    

    paveljanik commented at 5:28 am on May 24, 2016:

    Have you considered adding int64_t& time to the previous lookup? Both functions are almost the same…
11. 
    paveljanik commented at 5:29 am on May 24, 2016: contributor

    Concept ACK.
12. 
    arowser commented at 8:43 am on May 25, 2016: contributor

    Can one of the admins verify this patch?
13. Do not use mempool for GETDATA for tx accepted after the last mempool req.

    The ability to GETDATA a transaction which has not (yet) been relayed
     is a privacy loss vector.
    
    The use of the mempool for this was added as part of the mempool p2p
     message and is only needed to fetch transactions returned by it.

    7e908c7b82
14. laanwj merged this on May 31, 2016
15. laanwj closed this on May 31, 2016

    ---

16. laanwj referenced this in commit 862fd24b40 on May 31, 2016
17. codablock referenced this in commit 06ed9c7726 on Sep 16, 2017
18. codablock referenced this in commit f476adc2f8 on Sep 19, 2017
19. codablock referenced this in commit dfa0bd2341 on Dec 22, 2017
20. codablock referenced this in commit 8a97d7a7fd on Feb 7, 2018
21. UdjinM6 referenced this in commit 120893c63d on Feb 8, 2018
22. andvgal referenced this in commit d503ee3976 on Jan 6, 2019
23. andvgal referenced this in commit 1c3d1b7791 on Jan 6, 2019
24. CryptoCentric referenced this in commit 887c3672aa on Feb 28, 2019
25. CryptoCentric referenced this in commit b93921164c on Mar 2, 2019
26. furszy referenced this in commit 1f010c0969 on Jan 23, 2021
27. zkbot referenced this in commit 1d7ed06174 on Aug 13, 2021
28. zkbot referenced this in commit 56b5f95897 on Aug 17, 2021
29. MarcoFalke locked this on Sep 8, 2021

Contributors
gmaxwell jonasschnelli sipa petertodd paveljanik arowser

Labels
P2P Mempool