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

listen for new block hashes via ZMQ + fetch block templates via RPC #54

Merged
merged 30 commits into from Dec 19, 2023

Conversation

bernardoaraujor
Copy link
Contributor

@bernardoaraujor bernardoaraujor commented Oct 7, 2023

This PR makes the braidpool node able to listen for new block hashes via ZMQ, and fetch new block templates via RPC.
The node now takes some new CLI args, namely:

  • bitcoin: IP address for bitcoin node
  • rpcport: RPC port on bitcoin node
  • rpcuser: username for RPCs on bitcoin node
  • rpcpass: password for RPCs on bitcoin node
  • zmqport: ZMQ port on bitcoin node

We spawn a tokio thread called zmq::listener that listens to the ZMQ subscriptions. When a new hashblock notification comes in, block_template::fetcher calls the getblocktemplate RPC and sends the new block template into a mpsc channel.

A dummy placeholder tokio thread called block_template::consumer consumes the mpsc channel by simply printing the new block templates into the console.

@bernardoaraujor bernardoaraujor force-pushed the template-listener branch 7 times, most recently from dd9448c to b5c4bb3 Compare October 7, 2023 04:14
Copy link
Collaborator

@pool2win pool2win left a comment

Choose a reason for hiding this comment

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

Nice work. Left some nitty comments and some error handling fix requests.

We should move the mutinynet and docker setup files in a separate PR. Since these are dev tools, I am not sure yet if we should have these in the repo.

.gitignore Outdated Show resolved Hide resolved
node/src/cli.rs Outdated Show resolved Hide resolved
node/src/template.rs Outdated Show resolved Hide resolved
node/src/main.rs Outdated Show resolved Hide resolved
node/src/template.rs Outdated Show resolved Hide resolved
@bernardoaraujor
Copy link
Contributor Author

bernardoaraujor commented Oct 7, 2023

We should move the mutinynet and docker setup files in a separate PR. Since these are dev tools, I am not sure yet if we should have these in the repo.

From my experience working on this PR, mutinynet is really useful because it has a block time of ~30s, and waiting ~10min for new blocks on a development workflow is not viable. This will likely be the case for future development as well.

But I agree it's perhaps not ideal to impose this into the codebase as a whole, so mutinynet deployment files were moved over to https://github.com/bernardoaraujor/braidpool-mutinynet

If needed I can send another PR in the future bringing those over here or somewhere else.

@bernardoaraujor bernardoaraujor changed the title listen for new block templates via RPC + deploy local mutinynet node listen for new block templates via RPC Oct 7, 2023
@mcelrath
Copy link
Collaborator

mcelrath commented Oct 8, 2023

Thanks for this, good work! But polling bad. Bitcoin provides a ZMQ interface:

https://github.com/bitcoin/bitcoin/blob/master/doc/zmq.md

This will push block notifications to us.

We're going to be very sensitive to latency, with braid bead times that might be as low as 150ms.

How do you feel about rewriting this to use ZMQ?

@bernardoaraujor
Copy link
Contributor Author

bernardoaraujor commented Oct 11, 2023

There's a few different potential dependencies for this ZeroMQ implementation. Here's a summary of the tradeoffs for each option, ranked according to my perceived fitness the problem at hand:

  1. zmq: the most popular crate for ZMQ, and the most likely to recieve long-term maintenance. Doesn't provide async out-of-the-box, which forced me to slightly increase the complexity of the code by using things like tokio::task::spawn_blocking, Arc and Mutex.
  2. async_zmq: an async port of the crate mentioned above, which allows for simpler code. The project is relatively less popular than its sync counterpart, so I wonder if it could be abandoned in the future.
  3. bitcoincore-zmq: goes straight to the point (Bitcoin+ZMQ), but the project is very new and seems WIP (lots of TODOs on the README). Also non-async.
  4. bitcoin-net-zmq: also has the advantage of being a Bitcoin+ZMQ implementation. It is actually part of the larger effort bitcoin-rs, which is a WIP translation of C++ into Rust. There's no documentation so I didn't try to use it.
  5. futures-zmq: pretty much dead.
  6. tokio-zmq: pretty much dead.
  7. bitcoin-zmq: pretty much dead.

While searching for zmq on crates.io there's actually many other options. But most of them follow the patterns described above (low adoption, not actively maintained).

If I were to choose the best dependency for the long term, option 1 seems the most appealing to me. Option 2 is also a viable path. Please let me know your thoughts @mcelrath @pool2win

I pushed an implementation for option 1 on this PR. Alternatively, I also wrote an implementation for option 2, currently available on template-listener-async-zmq branch of my fork for comparison. It is slightly simpler code, but as mentioned above, with the tradeoff of a less popular dependency.

@bernardoaraujor
Copy link
Contributor Author

bernardoaraujor commented Oct 11, 2023

another question that comes to mind is:

will braidpool nodes also need to listen for other ZMQ topics aside from hashblock?

asking because of this

basically, the current implementation simply gets a ZMQ notification and uses that as a trigger for the getblocktemplate RPC.
since we're not subscribed to any other ZMQ topics aside from hashblock, the notification message is not being deserialized nor matched against any filter

if the braidpool node also needs to subscribe to other ZMQ notification topics, the code will need to deserialize the notification message and make sure that different topics are triggering the correct actions

@bernardoaraujor bernardoaraujor changed the title listen for new block templates via RPC listen for new block hashes via ZMQ + fetch block templates via RPC Oct 12, 2023
@bernardoaraujor bernardoaraujor force-pushed the template-listener branch 2 times, most recently from 8b45b92 to 5b68620 Compare October 21, 2023 03:03
@bernardoaraujor bernardoaraujor force-pushed the template-listener branch 2 times, most recently from 4cda461 to 1af423c Compare October 21, 2023 04:02
@bernardoaraujor
Copy link
Contributor Author

bernardoaraujor commented Oct 22, 2023

after going back and forth with @antonilol, the bitcoincore-zmq now supports a new async feature via this PR

so I'm switching to bitcoincore-zmq as dependency and I'm archiving my tokio-bitcoincore-zmq fork.

@mcelrath
Copy link
Collaborator

tested

So the standard way to authenticate to bitcoind is to use cookie authentication, which is the file ~/.bitcoin/.cookie and allows the local user to use bicoind without rpcuser and rpcpass. Looks like we can do this:

https://github.com/rust-bitcoin/rust-bitcoincore-rpc/blob/master/client/src/client.rs#L198

Before I merge this, can you get cookie auth working by default (and optional rpcuser/rpcpass)?

@mcelrath
Copy link
Collaborator

Could you add some debugging around the zmq connection? I didn't have zmq enabled in bitcoind and got no complaints from the braipdool node. e.g. fail if we can't connect to zmq.

Secondly can you add some docs around configuring the bitcoin node to publish zmq? I think it's just

zmqpubrawblock=tcp://localhost:28332

Do you understand the "high water mark" config options bitcoin has for zmq?

@antonilol
Copy link

antonilol commented Oct 26, 2023

Could you add some debugging around the zmq connection? I didn't have zmq enabled in bitcoind and got no complaints from the braipdool node. e.g. fail if we can't connect to zmq.

this is a problem i have had with the zmq crate (as a dependency of my bitcoincore-zmq crate). it appears to be a zmq protocol problem later fixed with a socket option (see https://stackoverflow.com/a/75893170/13800918), i will look into this

EDIT: maybe it is intentional: https://stackoverflow.com/a/14691718/13800918

EDIT2: it is intentional

@antonilol
Copy link

Do you understand the "high water mark" config options bitcoin has for zmq?

https://zeromq.org/socket-api/#high-water-mark

@antonilol
Copy link

i did not get it working with the socket options (ZMQ_HEARTBEAT_*, see http://api.zeromq.org/4-2:zmq-setsockopt), but after a lot of google searching and interrogating chat gpt i consulted the ultimate network debugging tool: wireshark. there is apparently a handshake between the subscriber and the publisher, where the subscriber could make up from if the publisher exists. the only problem is that i do not know if zmq exposes this (i think not). it might be possible but it could require patching zmq (only the subscriber, i do not consider any changes to the publisher a viable option)

@bernardoaraujor

This comment was marked as outdated.

@antonilol
Copy link

update on this comment: it is possible and libzmq exposes functionality for it, and the rust bindings do too.

i made a working proof of concept pr: antonilol/rust-bitcoincore-zmq#7, note that the stream will be reusable eventually, that is just not implemented yet

@plebhash plebhash mentioned this pull request Nov 6, 2023
@plebhash plebhash mentioned this pull request Nov 9, 2023
@bernardoaraujor
Copy link
Contributor Author

bernardoaraujor commented Nov 9, 2023

I realized that it's not safe to assume all zmq notifications would be coming from the same port.
The bitcoind node could be publishing different topics on different ports, which means we shouldn't have one single listener function and one single zmqport CLI arg.

So I adapted the code so that the listener function (now called zmq_hashblock_listener) only listens for hashblock notifications.

Also, the CLI arg is now called zmqhashblockport instead of zmqport.

Once we start implementing features based on different topics, we can add new functions and CLI args following a similar pattern.


Secondly can you add some docs around configuring the bitcoin node to publish zmq?

ack bernardoaraujor@e122da3


@mcelrath with regards to zmq debugging, it's probably better to wait until work on antonilol/rust-bitcoincore-zmq#7 is finished

I suggest we leave that for a follow-up PR, flagged for future reference here: #59

@mcelrath mcelrath merged commit ddb4c57 into braidpool:main Dec 19, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants