Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPC: Add new "getzmqnotifications" method #13570

Merged
merged 2 commits into from Jul 9, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+141 −15
Diff settings

Always

Just for now

Copy path View file
@@ -79,6 +79,8 @@ RPC changes
`getmempoolentry` when verbosity is set to `true` with sub-fields `ancestor`, `base`, `modified`
and `descendant` denominated in BTC. This new field deprecates previous fee fields, such as
`fee`, `modifiedfee`, `ancestorfee` and `descendantfee`.
- The new RPC `getzmqnotifications` returns information about active ZMQ
notifications.

External wallet files
---------------------
Copy path View file
@@ -193,7 +193,8 @@ BITCOIN_CORE_H = \
zmq/zmqabstractnotifier.h \
zmq/zmqconfig.h\
zmq/zmqnotificationinterface.h \
zmq/zmqpublishnotifier.h
zmq/zmqpublishnotifier.h \
zmq/zmqrpc.h


obj/build.h: FORCE
@@ -253,7 +254,8 @@ libbitcoin_zmq_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
libbitcoin_zmq_a_SOURCES = \
zmq/zmqabstractnotifier.cpp \
zmq/zmqnotificationinterface.cpp \
zmq/zmqpublishnotifier.cpp
zmq/zmqpublishnotifier.cpp \
zmq/zmqrpc.cpp
endif


Copy path View file
@@ -62,6 +62,7 @@

#if ENABLE_ZMQ
#include <zmq/zmqnotificationinterface.h>
#include <zmq/zmqrpc.h>
#endif

bool fFeeEstimatesInitialized = false;
@@ -99,10 +100,6 @@ void DummyWalletInit::AddWalletOptions() const
const WalletInitInterface& g_wallet_init_interface = DummyWalletInit();
#endif

#if ENABLE_ZMQ
static CZMQNotificationInterface* pzmqNotificationInterface = nullptr;
#endif

#ifdef WIN32
// Win32 LevelDB doesn't use filedescriptors, and the ones used for
// accessing block files don't count towards the fd_set size limit
@@ -279,10 +276,10 @@ void Shutdown()
g_wallet_init_interface.Stop();

#if ENABLE_ZMQ
if (pzmqNotificationInterface) {
UnregisterValidationInterface(pzmqNotificationInterface);
delete pzmqNotificationInterface;
pzmqNotificationInterface = nullptr;
if (g_zmq_notification_interface) {
UnregisterValidationInterface(g_zmq_notification_interface);
delete g_zmq_notification_interface;
g_zmq_notification_interface = nullptr;
}
#endif

@@ -1291,6 +1288,9 @@ bool AppInitMain()
*/
RegisterAllCoreRPCCommands(tableRPC);
g_wallet_init_interface.RegisterRPC(tableRPC);
#if ENABLE_ZMQ
RegisterZMQRPCCommands(tableRPC);
#endif

/* Start the RPC server already. It will be started in "warmup" mode
* and not really process calls already (but it will signify connections
@@ -1409,10 +1409,10 @@ bool AppInitMain()
}

#if ENABLE_ZMQ
pzmqNotificationInterface = CZMQNotificationInterface::Create();
g_zmq_notification_interface = CZMQNotificationInterface::Create();

if (pzmqNotificationInterface) {
RegisterValidationInterface(pzmqNotificationInterface);
if (g_zmq_notification_interface) {
RegisterValidationInterface(g_zmq_notification_interface);
}
#endif
uint64_t nMaxOutboundLimit = 0; //unlimited unless -maxuploadtarget is set
@@ -1,4 +1,4 @@
// Copyright (c) 2015-2017 The Bitcoin Core developers
// Copyright (c) 2015-2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

@@ -29,6 +29,15 @@ CZMQNotificationInterface::~CZMQNotificationInterface()
}
}

std::list<const CZMQAbstractNotifier*> CZMQNotificationInterface::GetActiveNotifiers() const
{
std::list<const CZMQAbstractNotifier*> result;

This comment has been minimized.

Copy link
@promag

promag Jul 1, 2018

Member

Could just return notifiers?

This comment has been minimized.

Copy link
@domob1812

domob1812 Jul 2, 2018

Author Contributor

No because notifiers is a list of non-const CZMQAbstractNotifier, and I'd rather not return the mutable objects. That's the reason why I originally had this "add to JSON" method - but I think that the additional copy done here is probably not relevant in practice.

for (const auto* n : notifiers) {
result.push_back(n);
}
return result;
}

CZMQNotificationInterface* CZMQNotificationInterface::Create()
{
CZMQNotificationInterface* notificationInterface = nullptr;
@@ -180,3 +189,5 @@ void CZMQNotificationInterface::BlockDisconnected(const std::shared_ptr<const CB
TransactionAddedToMempool(ptx);
}
}

CZMQNotificationInterface* g_zmq_notification_interface = nullptr;
@@ -1,4 +1,4 @@
// Copyright (c) 2015-2017 The Bitcoin Core developers
// Copyright (c) 2015-2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

@@ -18,6 +18,8 @@ class CZMQNotificationInterface final : public CValidationInterface
public:
virtual ~CZMQNotificationInterface();

std::list<const CZMQAbstractNotifier*> GetActiveNotifiers() const;

static CZMQNotificationInterface* Create();

protected:
@@ -37,4 +39,6 @@ class CZMQNotificationInterface final : public CValidationInterface
std::list<CZMQAbstractNotifier*> notifiers;
};

extern CZMQNotificationInterface* g_zmq_notification_interface;

#endif // BITCOIN_ZMQ_ZMQNOTIFICATIONINTERFACE_H
Copy path View file
@@ -0,0 +1,61 @@
// Copyright (c) 2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <zmq/zmqrpc.h>

#include <rpc/server.h>
#include <zmq/zmqabstractnotifier.h>
#include <zmq/zmqnotificationinterface.h>

#include <univalue.h>

namespace {

UniValue getzmqnotifications(const JSONRPCRequest& request)
{
if (request.fHelp || request.params.size() != 0) {
throw std::runtime_error(
"getzmqnotifications\n"
"\nReturns information about the active ZeroMQ notifications.\n"
"\nResult:\n"
"[\n"
" { (json object)\n"
" \"type\": \"pubhashtx\", (string) Type of notification\n"
" \"address\": \"...\" (string) Address of the publisher\n"
" },\n"
" ...\n"
"]\n"
"\nExamples:\n"
+ HelpExampleCli("getzmqnotifications", "")
+ HelpExampleRpc("getzmqnotifications", "")
);
}

UniValue result(UniValue::VARR);
if (g_zmq_notification_interface != nullptr) {
for (const auto* n : g_zmq_notification_interface->GetActiveNotifiers()) {
UniValue obj(UniValue::VOBJ);
obj.pushKV("type", n->GetType());
obj.pushKV("address", n->GetAddress());
result.push_back(obj);
}
}

return result;
}

const CRPCCommand commands[] =
{ // category name actor (function) argNames
// ----------------- ------------------------ ----------------------- ----------
{ "zmq", "getzmqnotifications", &getzmqnotifications, {} },
};

} // anonymous namespace

void RegisterZMQRPCCommands(CRPCTable& t)
{
for (const auto& c : commands) {
t.appendCommand(c.name, &c);
}
}
Copy path View file
@@ -0,0 +1,12 @@
// Copyright (c) 2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_ZMQ_ZMQRPC_H
#define BITCOIN_ZMQ_ZMQRPC_H

class CRPCTable;

void RegisterZMQRPCCommands(CRPCTable& t);

#endif // BITCOIN_ZMQ_ZMRRPC_H
Copy path View file
@@ -0,0 +1,33 @@
#!/usr/bin/env python3
# Copyright (c) 2018 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test for the ZMQ RPC methods."""

from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal


class RPCZMQTest(BitcoinTestFramework):

address = "tcp://127.0.0.1:28332"

This comment has been minimized.

Copy link
@promag

promag Jul 4, 2018

Member

nit, inline this (only 2 times)?

This comment has been minimized.

Copy link
@domob1812

domob1812 Jul 5, 2018

Author Contributor

I actually like it better like this, even if it is only used in two places for now. And if we add a addzmqnotification later, it will be used in more.

But if you strongly prefer this to be inlined, I can of course do it.

This comment has been minimized.

Copy link
@promag

promag Jul 5, 2018

Member

No strong opinion.


def set_test_params(self):
self.num_nodes = 1
self.setup_clean_chain = True

def run_test(self):
self._test_getzmqnotifications()

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Jul 11, 2018

Member

The test should be skipped similar to interface_zmq.py. Otherwise it will fail.

This comment has been minimized.

Copy link
@domob1812

domob1812 Jul 11, 2018

Author Contributor

That is indeed a good point! Since this was already merged, should I create a new PR with this fix?

This comment has been minimized.

Copy link
@domob1812

domob1812 Jul 12, 2018

Author Contributor

I'll work on a fix and send in a PR with it as soon as I'm ready. Thanks again for spotting this!

This comment has been minimized.

Copy link
@domob1812

domob1812 Jul 12, 2018

Author Contributor

Created #13646.


def _test_getzmqnotifications(self):
self.restart_node(0, extra_args=[])
assert_equal(self.nodes[0].getzmqnotifications(), [])

self.restart_node(0, extra_args=["-zmqpubhashtx=%s" % self.address])
assert_equal(self.nodes[0].getzmqnotifications(), [
{"type": "pubhashtx", "address": self.address},
])


if __name__ == '__main__':
RPCZMQTest().main()
@@ -116,6 +116,7 @@
'feature_versionbits_warning.py',
'rpc_preciousblock.py',
'wallet_importprunedfunds.py',
'rpc_zmq.py',
'rpc_signmessage.py',
'feature_nulldummy.py',
'mempool_accept.py',
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.