[RPC] Add RPC long poll notifications by jonasschnelli · Pull Request #7949 · bitcoin/bitcoin

jonasschnelli · 2016-04-26T15:33:04Z

Reasons for another notification interface

Currently there is no interface that could be extended to "private" notification secured behind the authorization (like peers connected/disconnected or a new wallet relevant transaction notification)
HTTP long poll notifications are very easy to set up and require almost no dependencies
HTTP long poll notifications can easily pushed over the internet using httpd reverse proxy together with a popper authentication method (certs or http auth digest) together with TLS.
HTTP long poll would allow connecting applications to do all kinds of things with just a single communication channel (currently you need RPC & ZMQ for most use cases which would require VPN or a fancy multi port stunnel connection to broadcast the notification over the internet)

How does it work

The listener calls the pollnotification RPC command.
If no notifications are available, the RPC thread will idle for a given timeout (30s by default)
If a notification was fired during the 30 seconds, the longpoll call will be responded with the new notification(s)
The client/listener can immediately reconnect and wait again
If notifications are already in the queue, the pollnotification command will immediately response.
Notifications can't get lost (possible to lose them during http transfer and if one exceed the queue limit)

Downsides

JSON encoding overhead

New RPC calls

setregisterednotifications [<notificationtype>] (possible types are hashtx and hashblock)
getregisterednotifications
pollnotifications

Missing

More tests
Documentation

I'd like to use a such interface to work on a remote GUI (use case: GUI on your local desktop, node on a VPS).

laanwj · 2016-04-26T15:40:03Z

I like the concept of being able to listen for events through http, however I think this is severely limited by having server-side state, limiting the number of listeners to only one.

What I'd personally prefer is, instead of longpolling, to subscribe to a 'stream' of events (e.g. websocket or just chunked encoding), where the set of events to listen to is in the request. This avoids having to store any client-state in the server - at least for longer than the request lasts.

jonasschnelli · 2016-04-26T15:45:30Z

[... ] having server-side state, limiting the number of listeners to only one

Right. The current implementation limits to only one listener. Extending this PR so it would support a client chosen UUID would not be very complicated (a set of queues and a set of registered notification types). Clients could register notification types along with a client-chosen UUID.
I might extend this PR to support multiple listeners.

jonasschnelli · 2016-04-26T17:45:23Z

Added a commit that allows multiple clients at the same time.

The new RPC commands require now a clientUUID parameter (a per client unique string, ideally a UUID after RFC 4122). Bitcoind keeps a queue, sequence numbers and registered types per client.

There is currently not max client limit and no way to remove clients (though you can unregister all notification types but not empty the current queue).

jonasschnelli · 2016-05-06T09:53:11Z

Rebased.
Would be nice to get some concept NACKs/ACKs.

maflcko · 2016-05-06T10:10:47Z

qa/rpc-tests/rpcsignals.py

Nit: python3

jonasschnelli · 2016-05-12T10:54:32Z

Rebased.

laanwj · 2017-01-11T13:09:26Z

Gah we need to take a look at this again after 0.14 is released.

jonasschnelli · 2017-01-11T13:27:05Z

Yes. Sure. I'll try to re-base and overhaul this soon.

TheBlueMatt · 2017-09-28T16:47:06Z

Plan on rebasing this, or should just be closed?

jonasschnelli · 2017-09-28T16:57:42Z

I'm currently rewriting this... will be ready soon.

jonasschnelli · 2017-10-20T06:02:06Z

Overhauled and rebased.

This is still server based (server keeps track of what the client has) queue max size is currently 1024^2 and does only contain hashes of blocks or transactions.
Each notification comes with a sequence number to detect lost transactions (which then should trigger a "full-client-sync").

TheBlueMatt

Concept ACK, though should definitely get more general concept review. I think this could use a more explicit register/deregister process eg registernewclient [ThingsClientCaresAbout] -> provides UUID (instead of registering taking a client-provided UUID), then an explicit deregister which removes queues for that client, instead of setting notifications to 0 and the queues for that client simply leaking.

TheBlueMatt · 2017-11-06T17:59:29Z

src/rpc/notifications.cpp

+    void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override
+    {
+        LOCK(m_cs_queue_manager);
+        BOOST_FOREACH (NotificationQueue* queue, m_map_sequence_numbers | boost::adaptors::map_values) {


Ugh, can we not use more BOOST_FOREACH garbage? Should be really easy to rewrite this without, no?

TheBlueMatt · 2017-11-06T18:00:45Z

src/rpc/notifications.cpp

+        }
+    }
+
+    void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override


I dont think this is the notification we want here - dont we want to use BlockConnected/Disconnected to notify clients of all connected blocks, not just new tips after reorgs which potentially connect multiple blocks?

TheBlueMatt · 2017-11-06T18:02:48Z

src/rpc/notifications.cpp

+{
+public:
+    std::deque<NotificationEntry> m_queue;
+    std::map<NotificationType, int32_t> m_map_sequence_numbers;


I generally prefer 64 bit ints here - sure, its unlikely you'd overflow 32 bits, but if you're online for 3 or 4 years you may start getting closeish.

TheBlueMatt · 2017-11-06T18:15:14Z

src/rpc/notifications.cpp

+        size_t queueSize = m_queue.size();
+        if (queueSize > MAX_QUEUE_SIZE) {
+            m_queue.pop_front();
+            LogPrintf("RPC Notification limit has been reached, dropping oldest element\n");


I'd be somwhat worried we'd fill someone's drive with debug log entries in case they forget to deregister a listener, here.

TheBlueMatt · 2017-11-06T18:29:11Z

src/rpc/notifications.cpp

+{
+public:
+    CCriticalSection m_cs_queue_manager;
+    std::map<clientUUID_t, NotificationQueue*> m_map_sequence_numbers;


It'd be great if we could de-duplicate the queues here - no need to have a queue per client, just have a global queue and keep track of how far delayed all the clients are in terms of the sequence number and just clean things up to the furthest-back client.

Also, please use unique_ptr here instead of manual management and maybe remove the queue when there are no registered types (and, I suppose, the client is caught up) instead of keeping around a null queue.

TheBlueMatt · 2017-11-06T18:34:53Z

src/rpc/notifications.cpp

+        }
+        if (startTime + timeOut + (500 / 1000.0) < GetTime())
+            break;
+        MilliSleep(500);


Should probably use a CV and a way for Interrupt to interrupt it instead of calling ShutdownRequested in a 500ms loop.

TheBlueMatt · 2017-11-06T18:51:37Z

Also, looks like the test is failing.

NicolasDorier · 2018-01-10T16:10:46Z

Strong Concept ACK for this one !
@jonasschnelli any idea if you will bring this one from the dead?

jonasschnelli · 2018-01-10T18:50:10Z

@NicolasDorier
I think there is no consensus about an additional push channel... also, @sipa brought up the idea of having a push channel (could be long poll) that acts similar then listsinceblock where the server doesn't need to keep track of clients (keep a queue).
I haven't looked closer at this approach.

NicolasDorier · 2018-01-14T19:15:58Z

I implemented a similar solution in NBXplorer. Basically there is a GetUTXOs(xPub) call, this call replay all the transactions of the xpub in topological order to create the current utxo for this xpub. While playing the transactions, it hashes them along the way (the hash after each transaction is effectively the equivalent of a bookmark). Then the bookmark + the UTXO is sent back to the client.

The client process the result, then call again GetUTXOs(xPub, bookmark). The server does the same operation, replaying all transactions while calculating bookmarks along the way, when it reaches the bookmark passed by the client, it knows that what is after is a differential to the previous bookmark. If there is no differential, it just long poll. If there is a differential, it sends it back to the client.

If the bookmark in parameter was not reached, then the full UTXO is sent back again to the client, with a flag indicating it is not a differential.

This solution does not involve server side state.

practicalswift · 2018-09-02T18:37:16Z

src/rpc/notifications.cpp

+static const char* MSG_HASHBLOCK = "hashblock";
+static const char* MSG_HASHTX = "hashtx";
+
+/* keep the max queue size large becase we don't


Typo found by codespell: becase

practicalswift · 2018-09-02T18:37:40Z

src/rpc/notifications.cpp

+
+    // populates a json object with all notifications in the queue
+    // returns a range to allow removing the elements from the queue
+    // after successfull transmitting


Typo found by codespell: successfull

promag

@jonasschnelli do you plan to pick this again?

promag · 2018-11-08T15:28:28Z

src/rpc/notifications.cpp

+            "\"  ,...\n"
+            "\"]\"\n"
+            "\nExamples:\n"
+            "\nCreate a transaction\n" +


jonasschnelli added the RPC/REST/ZMQ label Apr 26, 2016

jonasschnelli force-pushed the 2016/04/rpc_signals branch 2 times, most recently from e5734a6 to 6958509 Compare April 26, 2016 17:47

jonasschnelli force-pushed the 2016/04/rpc_signals branch from 6958509 to 90b28e4 Compare May 6, 2016 09:52

maflcko reviewed May 6, 2016
View reviewed changes

jonasschnelli force-pushed the 2016/04/rpc_signals branch 2 times, most recently from df5034a to 6dce696 Compare May 12, 2016 09:01

jonasschnelli force-pushed the 2016/04/rpc_signals branch from 6dce696 to 8660db9 Compare July 8, 2016 12:40

pstratem mentioned this pull request Jul 12, 2016

Block postprocessing is (relatively) slow. #8333

Closed

jonasschnelli mentioned this pull request Sep 9, 2016

Address Travis spurious failures #8680

Merged

jonasschnelli mentioned this pull request Feb 12, 2017

[RPC] Getting confirmations command #9745

Closed

laanwj added this to the 0.15.0 milestone Mar 5, 2017

laanwj modified the milestones: 0.16.0, 0.15.0 Jul 11, 2017

jonasschnelli force-pushed the 2016/04/rpc_signals branch 2 times, most recently from 7258d7c to 2813628 Compare October 20, 2017 05:59

jonasschnelli mentioned this pull request Oct 23, 2017

ZMQ: add publishers for wallet transactions. #10554

Closed

jonasschnelli added 2 commits October 22, 2017 15:28

Add RPC long poll notifications

ca6a3f8

[QA] Add rpcnotification test

2652c55

jonasschnelli force-pushed the 2016/04/rpc_signals branch from 2813628 to 2652c55 Compare October 23, 2017 01:32

TheBlueMatt reviewed Nov 6, 2017

View reviewed changes

NicolasDorier mentioned this pull request Jan 10, 2018

[RPC] Remove waitforblock and waitfornewblock #10519

Closed

laanwj modified the milestones: 0.16.0, 0.17.0 Jan 11, 2018

NicolasDorier mentioned this pull request Mar 8, 2018

Add waitfor RPC methods nopara73/MagicalCryptoWallet#65

Merged

jonasschnelli mentioned this pull request May 17, 2018

Wallet/RPC: Add listsincetx method with a stateless (server-side) long polling option #13262

Closed

2 tasks

maflcko added the Needs rebase label Jun 6, 2018

jonasschnelli modified the milestones: 0.17.0, 0.18.0 Jul 19, 2018

practicalswift reviewed Sep 2, 2018

View reviewed changes

promag reviewed Nov 30, 2018

View reviewed changes

src/rpc/notifications.cpp

"\" ,...\n"

"\"]\"\n"

"\nExamples:\n"

"\nCreate a transaction\n" +

Copy link

Contributor

promag Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

laanwj removed this from the 0.18.0 milestone Feb 6, 2019

fanquake added the Up for grabs label Jun 17, 2019

fanquake closed this Jun 17, 2019

laanwj removed the Needs rebase label Oct 24, 2019

jonasschnelli mentioned this pull request Nov 10, 2019

GUI: Who is the GUI being designed for and how do we better avoid bikeshedding? #17395

Closed

Bosch-0 mentioned this pull request Aug 5, 2020

Who is the GUI being designed for? -Continued Discussion- bitcoin-core/gui#45

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

maflcko added Up for grabs and removed Up for grabs labels Jul 24, 2025

Conversation

jonasschnelli commented Apr 26, 2016

Reasons for another notification interface

How does it work

Downsides

New RPC calls

Missing

Uh oh!

laanwj commented Apr 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Apr 26, 2016

Uh oh!

jonasschnelli commented Apr 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented May 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented May 12, 2016

Uh oh!

laanwj commented Jan 11, 2017

Uh oh!

jonasschnelli commented Jan 11, 2017

Uh oh!

TheBlueMatt commented Sep 28, 2017

Uh oh!

jonasschnelli commented Sep 28, 2017

Uh oh!

jonasschnelli commented Oct 20, 2017

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Nov 6, 2017

Uh oh!

NicolasDorier commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Jan 10, 2018

Uh oh!

NicolasDorier commented Jan 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

laanwj commented Apr 26, 2016 •

edited

Loading

jonasschnelli commented Apr 26, 2016 •

edited

Loading

NicolasDorier commented Jan 10, 2018 •

edited

Loading

NicolasDorier commented Jan 14, 2018 •

edited

Loading