Blockstream / electrs

An efficient re-implementation of Electrum Server in Rust
MIT License
321 stars 131 forks source link

Introduce Zmq and interrupt the main loop wait if zmq is setup and a block hash notification is received #111

Closed RCasatta closed 18 hours ago

RCasatta commented 2 months ago

In a follow up MR we can also listen to transactions from zmq so that the mempool update already finds transactions and doesn't need to fetch most of them from the node (probably needs a tx cache #110 )

shesek commented 2 months ago

In a follow up MR we can also listen to transactions from zmq ...

There's an WIP implementation of this I wrote awhile ago in this branch.

It has basic working functionality for updating the mempool view with individual transactions as they arrive from zmq rather than doing full periodic updates, but is still incomplete and missing some important optimizations (in particular for updating the mempool scripthash history indexes, which was already inefficient but is in a much hotter code patch with this change).

mempool update already finds transactions and doesn't need to fetch most of them from the node (probably needs a tx cache https://github.com/Blockstream/electrs/pull/110 )

This can be independent of a tx cache if you patch the mempool view directly with new transactions, instead of keeping them around but only applying them on the next periodic update.


My understanding is that in the current blockstream.info setup, there's a ZMQ client running alongside electrs that receives blockhash notifications from bitcoind and issues SIGUSR1 signals to electrs. So in a way, we already are utilizing ZMQ to subscribe to block updates.

Given this, is there much of an advantage to moving the blockhash zmq listener into the electrs daemon?

I do see a significant advantage to using zmq for updating the mempool with individual transactions, both for performance and to keep the electrs mempool view more closely in-sync with bitcoind's, without the latency introduced by periodic updates.

However if that's our goal, perhaps we should aim straight for an MR that includes it too? I'm not sure that integrating zmq is worth it just for block updates, and it might be beneficial to design the zmq integration with mempool updates in mind from the get go.

RCasatta commented 2 months ago

Given this, is there much of an advantage to moving the blockhash zmq listener into the electrs daemon?

Yes, so we can get rid of a service in the infra written in go that listen to zmq and send signals, it simplify the infra to have this logic in electrs instead.

However if that's our goal, perhaps we should aim straight for an MR that includes it too?

We would need zmq thread similar to this for it? This seems a step in that direction and also useful to get rid of a part of the infra (the service with the purpose of sending signals). But I am fine also if we want to go with the branch instead (adding also the wakeup in the loop) if it can be done in a reasonable timeframe.

shesek commented 1 month ago

We would need zmq thread similar to this for it? .. But I am fine also if we want to go with the branch instead

Right, we do. If we do just blocks subscription, then there are some things I slightly prefer in the branch (using an optional feature flag and the ZmqSyncer encapsulation) but none are critical and we could go with this PR too.

(adding also the wakeup in the loop)

In the branch there no longer is a sync loop when ZMQ is enabled, it relies on the subscriptions instead of doing periodic updates:

https://github.com/shesek/electrs/blob/b071ea9c5d37a8855e1934137cbe19d07a65a890/src/bin/electrs.rs#L108-L118

if it can be done in a reasonable timeframe.

Well define reasonable :) ZMQ-based mempool syncing is quite more involved than just block subscription so it would take longer to get right and require more throughout testing.

The upside is big and worth it imo, but if doing just block subscription to get rid of the zmq service is useful on its own then I wouldn't hold it up for that.