zmq: Add support to listen on multiple interfaces #18309

1. 
   n-thumann commented at 3:42 PM on March 10, 2020: contributor

   This PR adds support for ZeroMQ to listen on multiple interfaces, just like the RPC server. Currently, if you specify more than one e.g. zmqpubhashblock paramter, only the first one will be used. Therefore a user may be forced to listen on all interfaces (e.g. zmqpubhashblock=0.0.0.0:28332), which can result in an increased attack surface. With this PR a user can specify multiple interfaces to listen on, e.g. -zmqpubhashblock=tcp://127.0.0.1:28332 -zmqpubhashblock=tcp://192.168.1.123:28332.

2. DrahtBot added the label RPC/REST/ZMQ on Mar 10, 2020
3. 
   DrahtBot commented at 8:40 PM on March 10, 2020: member

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   No conflicts as of last run.

4. 
   promag commented at 9:02 PM on March 10, 2020: member

   Concept ACK.

   Maybe use a minimal diff

        for (const auto& entry : factories)
        {
            std::string arg("-zmq" + entry.first);
   -        if (gArgs.IsArgSet(arg))
   -        {
   -            CZMQNotifierFactory factory = entry.second;
   -            std::string address = gArgs.GetArg(arg, "");
   +        CZMQNotifierFactory factory = entry.second;
   +        for (const std::string& address : gArgs.GetArgs(arg)) {
                CZMQAbstractNotifier *notifier = factory();
                notifier->SetType(entry.first);
                notifier->SetAddress(address);
   

   Also:

   • update the doc/zmq.md with this change
   • add release note too
   • update some test in test/functional/interface_zmq.py with multiple interfaces.

   Welcome!

5. n-thumann force-pushed on Mar 10, 2020
6. 
   promag commented at 8:05 AM on March 11, 2020: member

   • update some test in test/functional/interface_zmq.py with multiple interfaces. @n-thumann don't forget about this.

7. 
   n-thumann commented at 8:59 AM on March 11, 2020: contributor

   • update some test in test/functional/interface_zmq.py with multiple interfaces.

   @n-thumann don't forget about this.

   Thank you for your feedback! The test is already in the works :)

8. 
   n-thumann commented at 6:16 PM on March 11, 2020: contributor

   Currently the debug.log prints 2020-03-11T17:52:01.715152Z [scheduler] zmq: Publish hashblock <blockhash> 2020-03-11T17:52:01.715248Z [scheduler] zmq: Publish hashblock <blockhash>, when hashblock is triggered and ZeroMQ is listening on two interfaces. Do you think it´s useful to append something like to tcp://127.0.0.1:28332 to each log entry, so that a user knows why there is an apparently duplicate entry?

9. 
   promag commented at 6:31 PM on March 11, 2020: member

   Do you think it´s useful to append something like to tcp://127.0.0.1:28332 to each log entry, so that a user knows why there is an apparently duplicate entry?

   Yes, makes sense in this change. Alternatively it could avoid printing the duplicate messages but not worth it.

10. in test/functional/interface_zmq.py:51 in 9e4685168f outdated

      47 | @@ -48,6 +48,7 @@ def run_test(self):
      48 |          try:
      49 |              self.test_basic()
      50 |              self.test_reorg()
      51 | +            self.test_multiple_interfaces();
    

    ---

    ---

    

    promag commented at 6:38 PM on March 11, 2020:

    test/functional/interface_zmq.py:51:44: E703 statement ends with a semicolon
    ^---- failure generated from test/lint/lint-python.sh
    

    ---

    

    n-thumann commented at 6:40 PM on March 11, 2020:

    Fixed :)

11. n-thumann force-pushed on Mar 11, 2020
12. n-thumann force-pushed on Mar 11, 2020
13. n-thumann requested review from promag on Mar 16, 2020
14. laanwj added the label Feature on Mar 19, 2020
15. 
    mruddy commented at 6:49 PM on April 21, 2020: contributor

    Tested ACK

    I tested on Ubuntu 19.10 x86_64 with zmq version 4.3.2.

    I verified that the node starts when not specifying any of the zmq flags, when starting with between 1 and 3 -zmqpubhashblock on different addresses and/or ports, with and without -zmqpubhashblockhwm=10000. I verified log entries, and I also verified receiving messages with the contrib/zmq/zmq_sub.py subscriber. @n-thumann side note, you might have the right skill set to review my #14687 related to zmq, if you have some spare cycles. that's a small change that the lightning guys have been requesting for a while. thanks!

16. DrahtBot added the label Needs rebase on Aug 31, 2020
17. n-thumann force-pushed on Aug 31, 2020
18. DrahtBot removed the label Needs rebase on Aug 31, 2020
19. 
    instagibbs commented at 1:47 AM on September 2, 2020: member

    concept ACK, will review

20. in test/functional/interface_zmq.py:221 in 5f46f63c17 outdated

     216 | +            socket.set(zmq.RCVTIMEO, 60000)
     217 | +            hashblock = ZMQSubscriber(socket, b"hashblock")
     218 | +            socket.connect(address)
     219 | +            subscribers.append({'address': address, 'socket': socket, 'hashblock': hashblock})
     220 | +
     221 | +        self.restart_node(0, ['-zmqpub%s=%s' % (subscribers[0]['hashblock'].topic.decode(), subscribers[0]['address']),
    

    ---

    ---

    

    instagibbs commented at 2:15 AM on September 2, 2020:

    maybe compress this into:

    self.restart_node(0, ['-zmqpub%s=%s' % (subscribers[i]['hashblock'].topic.decode(), subscribers[i]['address']) for i in range(2)])

    or there-about(didn't check parenthesis lining up :) )

    ---

    

    n-thumann commented at 8:18 AM on September 2, 2020:

    Thanks, applied your change slightly modified along with some other minor changes :)

21. instagibbs approved
22. 
    instagibbs commented at 2:17 AM on September 2, 2020: member

    utACK https://github.com/bitcoin/bitcoin/pull/18309/commits/6b8d300d2560daa80d6575cb047588aec76aa5dc

    Small, straight forward change. We can clean up log messages later if they're confusing.

23. n-thumann force-pushed on Sep 2, 2020
24. 
    instagibbs commented at 12:56 PM on September 2, 2020: member

    re-utACK https://github.com/bitcoin/bitcoin/pull/18309/commits/ab996c2a9ecd3f75fb4621f87d9303bd18e891a2

25. 
    jonatack commented at 12:08 PM on September 4, 2020: member

    Concept ACK

26. DrahtBot added the label Needs rebase on Sep 19, 2020
27. n-thumann force-pushed on Sep 20, 2020
28. DrahtBot removed the label Needs rebase on Sep 20, 2020
29. DrahtBot added the label Needs rebase on Sep 23, 2020
30. zmq: Add support to listen on multiple interfaces 347c94f551
31. doc: Adjust ZMQ usage to support multiple interfaces b1c3f180ec
32. doc: Add release notes to support multiple interfaces a0b2e5cb6a
33. n-thumann force-pushed on Sep 23, 2020
34. DrahtBot removed the label Needs rebase on Sep 23, 2020
35. n-thumann force-pushed on Sep 30, 2020
36. 
    n-thumann commented at 10:05 PM on September 30, 2020: contributor

    Adjusted log output to new notification types of #19572.

37. test: Add zmq test to support multiple interfaces 241803da21
38. zmq: Append address to notify log output e66870c5a4
39. n-thumann force-pushed on Sep 30, 2020
40. luke-jr approved
41. 
    luke-jr commented at 3:29 PM on October 1, 2020: member

    utACK

42. 
    laanwj commented at 3:42 PM on October 1, 2020: member

    Code review ACK e66870c5a4c2adbd30dca67d409fd5cd98697587

43. 
    instagibbs commented at 3:42 PM on October 1, 2020: member

    reACK https://github.com/bitcoin/bitcoin/pull/18309/commits/e66870c5a4c2adbd30dca67d409fd5cd98697587

44. laanwj merged this on Oct 1, 2020
45. laanwj closed this on Oct 1, 2020

    ---

46. sidhujag referenced this in commit eee79cf700 on Oct 4, 2020
47. Fabcien referenced this in commit 8b7ad52348 on Oct 14, 2021
48. DrahtBot locked this on Feb 15, 2022

Contributors

n-thumannDrahtBotpromagmruddyinstagibbsjonatackluke-jrlaanwj

Review Requested
promag