ZMQ: Support UNIX domain sockets #27679

pinheadmz · 2023-05-16T20:08:15Z

This is a follow-up to #27375, allowing ZMQ notifications to be published to a UNIX domain socket.

Fortunately, libzmq handles unix sockets already, all we really have to do to support it is allow the format in the actual option.

libzmq uses the prefix ipc:// as opposed to unix: which is used by Tor and now also by bitcoind so we need to switch that internally.

As far as I can tell, LND supports ipc:// and unix:// (notice the double slashes).

With this patch, LND can connect to bitcoind using unix sockets:

Example:

bitcoin.conf:

zmqpubrawblock=unix:/tmp/zmqsb
zmqpubrawtx=unix:/tmp/zmqst

lnd.conf:

bitcoind.zmqpubrawblock=ipc:///tmp/zmqsb
bitcoind.zmqpubrawtx=ipc:///tmp/zmqst

DrahtBot · 2023-05-16T20:08:18Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	laanwj, guggero, tdb3, achow101
Concept ACK	TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29432 (Stratum v2 Template Provider (take 3) by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

TheCharlatan · 2023-05-16T21:07:01Z

Concept ACK

pinheadmz · 2023-05-30T20:02:28Z

🐙 This pull request conflicts with the target branch and needs rebase.

Relax bot! sheesh. I'm gonna wait until #27375 gets approved

src/init.cpp

src/netaddress.h

pinheadmz · 2023-10-25T13:55:33Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Waiting for #27375

567cec9 doc: add release notes and help text for unix sockets (Matthew Zipkin) bfe5192 test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin) c65c0d0 init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin) c3bd431 gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin) a88bf9d i2p: construct Session with Proxy instead of CService (Matthew Zipkin) d9318a3 net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin) ac2ecf3 proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin) a89c3f5 netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin) 3a7d654 net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin) 74f568c netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin) bae86c8 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin) adb3a3e configure: test for unix domain sockets (Matthew Zipkin) Pull request description: Closes #27252 UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system. There has been work on [unix domain sockets before](#9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=` I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example): `tor --SocksPort unix:/Users/matthewzipkin/torsocket/x` `bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x` Prep work for this feature includes: - Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService` - Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path) Future work: - Enable UNIX sockets for ZMQ (#27679) - Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet) - Enable UNIX sockets on windows where supported - Update Network Proxies dialog in GUI to support UNIX sockets ACKs for top commit: Sjors: re-ACK 567cec9 tdb3: re ACK for 567cec9. achow101: ACK 567cec9 vasild: ACK 567cec9 Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1

DrahtBot · 2024-03-13T16:44:18Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/22614485115}

pinheadmz · 2024-03-13T16:57:51Z

src/init.cpp

+        {"-i2psam",                 false},
+        {"-onion",                  true},
+        {"-proxy",                  true},
+        {"-rpcbind",                false},
+        {"-torcontrol",             false},
+        {"-whitebind",              false},
+        {"-zmqpubhashblock",        true},
+        {"-zmqpubhashtx",           true},
+        {"-zmqpubrawblock",         true},
+        {"-zmqpubrawtx",            true},
+        {"-zmqpubsequence",         true}


unix socket support for i2psam and rpcbind will be implemented in future work, so this little matrix is anticipating that

laanwj · 2024-04-16T12:26:00Z

src/zmq/zmqnotificationinterface.cpp

-        for (const std::string& address : gArgs.GetArgs(arg)) {
+        for (std::string& address : gArgs.GetArgs(arg)) {
+            // libzmq uses prefix "ipc://" for UNIX domain sockets
+            ReplaceAll(address, ADDR_PREFIX_UNIX, ADDR_PREFIX_IPC);


Is ReplaceAll the right function to use here?
Sure, it's unlikely for it to appear multiple times, but from a principle of least surprise angle, it'd make sense to only replace the prefix.

pinheadmz · 2024-04-16T17:51:04Z

Rebasing on master since one commit was merged separately in #29649

pinheadmz · 2024-04-16T18:16:21Z

Thanks so much for your review @laanwj addressed your comments

laanwj · 2024-04-17T11:05:31Z

Looks good to me now
Code review ACK 21d0e6c

tdb3

crACK for 21d0e6c. Changes lgtm. Will follow up with some testing within the next few days as time allows.

guggero

Tested and code review ACK 21d0e6c

Thanks a lot for the fix!

tdb3 · 2024-04-19T01:24:34Z

crACK for 21d0e6c. Changes lgtm. Will follow up with some testing within the next few days as time allows.

re ACK for 21d0e6c

Retested the following:

using "ipc:///tmp/mysocket" form in bitcoin.conf causes error and quit (as expected, since it's not unix:/)
using "notvalid:///tmp/sock" in bitcoin.conf causes error and quit (as expected, since it's not unix:/)
Running with LND 0.17.4 on signet with UNIX sockets for ZMQ (bitcoin.conf and lnd.conf as described above in ZMQ: Support UNIX domain sockets #27679 (review))

p.s. Looks like the invalid port error message is duplicated.

2024-04-19T01:35:55Z init message: Loading banlist…
2024-04-19T01:35:55Z SetNetworkActive: true
2024-04-19T01:35:55Z [error] Invalid port specified in -zmqpubrawtx: 'notvalid:///tmp/sock'
Error: Invalid port specified in -zmqpubrawtx: 'notvalid:///tmp/sock'
2024-04-19T01:35:55Z Shutdown: In progress...
2024-04-19T01:35:55Z scheduler thread exit
2024-04-19T01:35:55Z Flushed fee estimates to fee_estimates.dat.
2024-04-19T01:35:55Z Shutdown: done
dev@bdev01:~/bitcoin$

achow101 · 2024-04-22T15:11:51Z

ACK 21d0e6c

Based on Github-Pull: bitcoin#27679 Based on: c87b0a0

Github-Pull: bitcoin#27679 Rebased-From: 791dea2

fanquake · 2024-04-26T07:35:08Z

src/init.cpp

    }) {
-        for (const std::string& socket_addr : args.GetArgs(port_option)) {
+        const std::string arg{port_option.first};
+        const bool unix{port_option.second};


FYI https://github.com/bitcoin/bitcoin/runs/24257306970:

init.cpp: In function ‘bool AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)’: init.cpp:1319:20: error: unused variable ‘unix’ [-Werror=unused-variable] 1319 | const bool unix{port_option.second}; | ^~~~

Fixed in #29968

No warnings for me locally using

x86_64-w64-mingw32-g++-posix (GCC) 13-posix

No warnings for me locally using

Did you turn on the relevant warning flag

No warnings for me locally using

Did you turn on the relevant warning flag

My bad! 🤦

fa9abf9 refactor: Avoid unused-variable warning in init.cpp (MarcoFalke) Pull request description: Fixes #27679 (comment) ACKs for top commit: TheCharlatan: ACK fa9abf9 Tree-SHA512: dcf56d7aa68578ba611a2dc591de036ab1d08f7f4dfb35d325ecf7241d8e17abc0af7005b96c44da9777adc36961b4da7fdde282a1ab0e0a6f229c8108923101

, bitcoin#29649, bitcoin#27679, bitcoin#29968, partial bitcoin#26312 (UNIX domain sockets support) 10c7c1d merge bitcoin#29968: Avoid unused-variable warning in init.cpp (Kittywhiskers Van Gogh) 82a0fb2 merge bitcoin#27679: Support UNIX domain sockets (Kittywhiskers Van Gogh) a23d651 merge bitcoin#29649: remove unnecessary log message (Kittywhiskers Van Gogh) 343bf83 merge bitcoin#27375: support unix domain sockets for -proxy and -onion (Kittywhiskers Van Gogh) 0fd83f3 merge bitcoin#28695: Sanity check private keys received from SAM proxy (Kittywhiskers Van Gogh) 6e55ab5 partial bitcoin#26312: Remove Sock::Get() and Sock::Sock() (Kittywhiskers Van Gogh) 859da12 merge bitcoin#22087: Validate port-options (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #6630 ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 10c7c1d Tree-SHA512: cf3bb84a34315fd188bf1427f59e8b050a04f26d2d4a152a8477c50daef154b35e0f29e49787ac8b585078e985d7192bc39bde27ea39a3b547c297cb2fc3a2ae

DrahtBot added the RPC/REST/ZMQ label May 16, 2023

DrahtBot added the CI failed label May 16, 2023

pinheadmz force-pushed the zmq-unix-domain-socket branch from 0e2d156 to a584969 Compare May 17, 2023 17:14

DrahtBot removed the CI failed label May 17, 2023

DrahtBot mentioned this pull request May 18, 2023

net: support unix domain sockets for -proxy and -onion #27375

Merged

DrahtBot added the Needs rebase label May 30, 2023

pinheadmz mentioned this pull request Jul 6, 2023

exclude ipc scheme from port check #28020

Closed

luke-jr reviewed Jul 27, 2023

View reviewed changes

src/init.cpp Show resolved Hide resolved

luke-jr reviewed Jul 27, 2023

View reviewed changes

src/netaddress.h Outdated Show resolved Hide resolved

guggero mentioned this pull request Dec 18, 2023

GitHub: use bitcoind v26.0 for CI lightningnetwork/lnd#8273

Merged

pinheadmz force-pushed the zmq-unix-domain-socket branch from a584969 to 95fda8a Compare March 13, 2024 14:32

DrahtBot removed the Needs rebase label Mar 13, 2024

pinheadmz force-pushed the zmq-unix-domain-socket branch from 95fda8a to 7b4c7bd Compare March 13, 2024 16:44

DrahtBot added the CI failed label Mar 13, 2024

pinheadmz marked this pull request as ready for review March 13, 2024 16:54

pinheadmz commented Mar 13, 2024

View reviewed changes

DrahtBot mentioned this pull request Mar 13, 2024

scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] #29641

Draft

DrahtBot removed the CI failed label Mar 13, 2024

laanwj reviewed Apr 16, 2024

View reviewed changes

pinheadmz force-pushed the zmq-unix-domain-socket branch from 7b4c7bd to 63cc219 Compare April 16, 2024 17:49

pinheadmz added 3 commits April 16, 2024 14:14

zmq: accept unix domain socket address for notifier

c87b0a0

test: cover unix sockets in zmq interface

791dea2

doc: release notes for PR 27679

21d0e6c

pinheadmz force-pushed the zmq-unix-domain-socket branch from 63cc219 to 21d0e6c Compare April 16, 2024 18:15

DrahtBot requested a review from tdb3 April 17, 2024 11:05

tdb3 reviewed Apr 17, 2024

View reviewed changes

guggero approved these changes Apr 18, 2024

View reviewed changes

guggero mentioned this pull request Apr 19, 2024

release: create v0.17.5 release branch lightningnetwork/lnd#8664

Merged

achow101 merged commit 04c90f1 into bitcoin:master Apr 22, 2024

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Apr 24, 2024

zmq: accept unix domain socket address for notifier

dabf896

Based on Github-Pull: bitcoin#27679 Based on: c87b0a0

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Apr 24, 2024

test: cover unix sockets in zmq interface

99a5703

Github-Pull: bitcoin#27679 Rebased-From: 791dea2

fanquake reviewed Apr 26, 2024

View reviewed changes

maflcko mentioned this pull request Apr 26, 2024

refactor: Avoid unused-variable warning in init.cpp #29968

Merged

pinheadmz mentioned this pull request Oct 31, 2024

Remove libevent as a dependency (HTTP / cli / torcontrol) #31194

Open

4 tasks

doitwithnotepad mentioned this pull request Dec 2, 2024

Add support for Unix domain sockets in ZeroMQ notifications cculianu/Fulcrum#267

Merged

xyephy mentioned this pull request Feb 26, 2025

Upgrade to bitcoin v28 whiveio/whive#12

Open

kwvg added a commit to kwvg/dash that referenced this pull request Apr 19, 2025

merge bitcoin#27679: Support UNIX domain sockets

a8633cd

kwvg added a commit to kwvg/dash that referenced this pull request Apr 19, 2025

merge bitcoin#27679: Support UNIX domain sockets

91d6e63

kwvg added a commit to kwvg/dash that referenced this pull request Apr 22, 2025

merge bitcoin#27679: Support UNIX domain sockets

cf16374

kwvg added a commit to kwvg/dash that referenced this pull request Apr 22, 2025

merge bitcoin#27679: Support UNIX domain sockets

82a0fb2

bitcoin locked and limited conversation to collaborators Apr 26, 2025

ZMQ: Support UNIX domain sockets #27679

ZMQ: Support UNIX domain sockets #27679

Uh oh!

Conversation

pinheadmz commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

TheCharlatan commented May 16, 2023

Uh oh!

pinheadmz commented May 30, 2023

Uh oh!

Uh oh!

Uh oh!

pinheadmz commented Oct 25, 2023

Uh oh!

DrahtBot commented Mar 13, 2024

Uh oh!

pinheadmz Mar 13, 2024

Choose a reason for hiding this comment

Uh oh!

laanwj Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

pinheadmz commented Apr 16, 2024

Uh oh!

pinheadmz commented Apr 16, 2024

Uh oh!

laanwj commented Apr 17, 2024

Uh oh!

tdb3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

tdb3 commented Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achow101 commented Apr 22, 2024

Uh oh!

fanquake Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

maflcko Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

hebasto Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

fanquake Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

hebasto Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pinheadmz commented May 16, 2023 •

edited

Loading

DrahtBot commented May 16, 2023 •

edited

Loading

tdb3 left a comment •

edited

Loading

tdb3 commented Apr 19, 2024 •

edited

Loading