Disable the mempool P2P command when bloom filters disabled #8078

1. 
   petertodd commented at 1:48 pm on May 20, 2016: contributor

   Only useful to SPV peers, and attackers… like bloom is a DoS vector as far more data is sent than received.
2. 
   jonasschnelli commented at 1:49 pm on May 20, 2016: contributor

   utACK fd432d7bb487e410a5e83f375df018132f183871
3. laanwj added the label P2P on May 20, 2016
4. 
   laanwj commented at 1:57 pm on May 20, 2016: member

   utACK/concept ACK https://github.com/bitcoin/bitcoin/pull/8078/commits/fd432d7bb487e410a5e83f375df018132f183871
5. 
   gmaxwell commented at 1:59 pm on May 20, 2016: contributor

   This should also make getdata only use the relaypool to close off the mempool privacy loss.
6. 
   petertodd commented at 2:05 pm on May 20, 2016: contributor

   @gmaxwell Sounds reasonable, but I think out of scope of this pull-req.
7. Disable the mempool P2P command when bloom filters disabled

   Only useful to SPV peers, and attackers... like bloom is a DoS vector as far
   more data is sent than received.

   beceac9bbf
8. 
   kazcw commented at 2:50 pm on May 20, 2016: contributor

   Would it make sense also to trigger this when bloom filters are enabled, but pfrom has not set one?
9. Add RPC test for the p2p mempool command in conjunction with disabled bloomfilters 3d3602faf4
10. 
    jonasschnelli commented at 2:52 pm on May 20, 2016: contributor

    Simple test you might want to pull into this PR: https://github.com/jonasschnelli/bitcoin/commit/3d3602faf4a47855be264f05d9d52253e8bd0f9d
11. petertodd force-pushed on May 20, 2016
12. 
    petertodd commented at 3:10 pm on May 20, 2016: contributor

    @jonasschnelli Good idea, done.
13. 
    luke-jr commented at 3:10 pm on May 20, 2016: member

    Also useful to bootstrap a mempool.
14. 
    petertodd commented at 3:21 pm on May 20, 2016: contributor

    @luke-jr That’s why it’s still allowed for whitelisted nodes; in general it’s a DoS attack and we need a better system. Also we discussed the bootstrap case and think getting all the mempool isn’t actually all that useful, for things we expect to need it like block relaying optimizations - need a subset.
15. in qa/rpc-tests/p2p-mempool.py: in 3d3602faf4

    71+        self.lastInv = []
    72+        self.send_message(msg_mempool())
    73+
    74+class P2PMempoolTests(BitcoinTestFramework):
    75+    def setup_chain(self):
    76+        initialize_chain_clean(self.options.tmpdir, 2)
    

    ---

    ---

    

    MarcoFalke commented at 2:18 pm on May 21, 2016:

    Could you use __init__ for that, instead of overwriting setup_chain?

    I.e. something like

    0    def __init__(self):
    1        super().__init__()
    2        self.num_nodes = 2
    3        self.setup_clean_chain = True # not needed (default should already be true)
    

16. 
    arowser commented at 8:43 am on May 25, 2016: contributor

    Can one of the admins verify this patch?
17. 
    MarcoFalke commented at 11:46 am on June 3, 2016: member

    @petertodd @jonasschnelli Interested in fixing the two nits for the rpc test?
18. 
    laanwj commented at 2:25 pm on June 7, 2016: member

    utACK https://github.com/bitcoin/bitcoin/pull/8078/commits/beceac9bbf14bf4a81f6f63b9cca2a64157054ae
19. 
    laanwj commented at 10:56 am on June 8, 2016: member

    Going to merge this - the nits can be done later.
20. laanwj merged this on Jun 8, 2016
21. laanwj closed this on Jun 8, 2016

    ---

22. laanwj referenced this in commit 2156fa23b8 on Jun 8, 2016
23. codablock referenced this in commit 3014543a5e on Sep 16, 2017
24. codablock referenced this in commit 9845bfe13a on Sep 19, 2017
25. codablock referenced this in commit 2ee817fb0d on Dec 22, 2017
26. andvgal referenced this in commit 4bad9e80cc on Jan 6, 2019
27. furszy referenced this in commit 297fd18fd4 on Feb 13, 2021
28. MarcoFalke locked this on Sep 8, 2021

Contributors
petertodd jonasschnelli laanwj gmaxwell kazcw luke-jr MarcoFalke arowser

Labels
P2P