fix: expose zmqpubhashblock port and clarify ZMQ stream labels - #1378
fix: expose zmqpubhashblock port and clarify ZMQ stream labels#1378chandachewe10 wants to merge 1 commit into
Conversation
fb582cb to
50d1216
Compare
Greptile SummaryThis PR exposes the
Confidence Score: 3/5Safe to merge once the migration is fixed; the rest of the changes are correct and the test coverage is solid. The port-exposure logic, Docker template update, type changes, and getOpenPorts extension are all correct. The one defect is in the v400 migration: it assigns the flat base port (28534) to every bitcoin node instead of base + node.id. For any existing network with two or more bitcoin nodes, every backend will display the same zmqHashBlock port in the ConnectTab until the network is restarted. getOpenPorts does silently fix the duplicates at start time, but the wrong values persist in the saved state and are visible to users in the interim. The fix is a one-token change; the rest of the PR is ready to go. src/utils/migrations.ts — the v400 migration needs a node.id offset on the zmqHashBlock assignment.
|
| Filename | Overview |
|---|---|
| src/utils/migrations.ts | Adds v400 migration for zmqHashBlock port; assigns the flat base port (28534) to every bitcoin node without the node.id offset, causing all nodes to show the same port in the ConnectTab until the next network start. |
| src/utils/network.ts | Adds zmqHashBlock to createBitcoindNetworkNode (correctly offset by id) and getOpenPorts conflict detection; both additions are consistent with the existing patterns. |
| src/utils/constants.ts | Adds zmqHashBlock: 28534 to BasePorts.bitcoind; the chosen base leaves a safe gap above zmqBlock (28334) and below zmqTx (29335) for any realistic number of nodes. |
| src/lib/docker/nodeTemplates.ts | Correctly adds zmqHashBlockPort parameter, exposes container port 28336 in the expose list, and maps it in the ports array; comments updated to distinguish raw from hash streams. |
| src/components/designer/bitcoin/ConnectTab.tsx | Adds zmqHashBlockHost row using the correct i18n key and port field; ordering is sensible alongside zmqBlock and zmqTx rows. |
| src/lib/docker/dockerService.spec.ts | Adds create390Network helper and migration test covering the v400 path; confirms zmqHashBlock is defined after migration. |
| src/utils/network.spec.ts | Adds zmqHashBlock assertions to all relevant getOpenPorts test cases and adds a dedicated test for zmqHashBlock conflict detection; coverage is comprehensive. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Host
participant Docker
participant Bitcoind
Note over Bitcoind: listens on<br/>28334 (zmqpubrawblock)<br/>28335 (zmqpubrawtx)<br/>28336 (zmqpubhashblock)
Host->>Docker: connect tcp://127.0.0.1:28334
Docker->>Bitcoind: forward → 28334 (rawblock)
Host->>Docker: connect tcp://127.0.0.1:29335
Docker->>Bitcoind: forward → 28335 (rawtx)
Host->>Docker: connect tcp://127.0.0.1:28534
Docker->>Bitcoind: forward → 28336 (hashblock) ← NEW
Note over Host,Bitcoind: LND / litd use rawblock (28334) + rawtx (28335)<br/>Eclair uses hashblock (28336) + rawtx (28335)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Host
participant Docker
participant Bitcoind
Note over Bitcoind: listens on<br/>28334 (zmqpubrawblock)<br/>28335 (zmqpubrawtx)<br/>28336 (zmqpubhashblock)
Host->>Docker: connect tcp://127.0.0.1:28334
Docker->>Bitcoind: forward → 28334 (rawblock)
Host->>Docker: connect tcp://127.0.0.1:29335
Docker->>Bitcoind: forward → 28335 (rawtx)
Host->>Docker: connect tcp://127.0.0.1:28534
Docker->>Bitcoind: forward → 28336 (hashblock) ← NEW
Note over Host,Bitcoind: LND / litd use rawblock (28334) + rawtx (28335)<br/>Eclair uses hashblock (28336) + rawtx (28335)
Reviews (1): Last reviewed commit: "fix: expose zmqpubhashblock port and cla..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
50d1216 to
287b731
Compare
|
@chandachewe10 thank you for this fix. However, I noticed the commits on this branch were pushed by |
The bitcoind daemon publishes three ZMQ streams:
- zmqpubrawblock on container port 28334
- zmqpubrawtx on container port 28335
- zmqpubhashblock on container port 28336
Previously only rawblock and rawtx were mapped to the host, making
the hashblock stream inaccessible from outside Docker. Additionally
the ConnectTab labels were ambiguous ('ZMQ Block Host'), not
distinguishing rawblock from hashblock.
Changes:
- Add zmqHashBlock port to BitcoindNode type and BasePorts constants
(base port 28534, so host ports start at 28534 for the first node)
- Expose container port 28336 in nodeTemplates.ts and map it to the
allocated host port in docker-compose
- Rename 'ZMQ Block Host' -> 'ZMQ Raw Block Host' and
'ZMQ Transaction Host' -> 'ZMQ Raw Transaction Host' in i18n strings
- Add 'ZMQ Hash Block Host' entry to the Bitcoin ConnectTab
- Add getOpenPorts() conflict detection for zmqHashBlock
- Add v400 migration to backfill zmqHashBlock for existing networks
- Update all affected tests and add new test cases
287b731 to
7355cb0
Compare
|
@Jem256 thanks that wasn't intentional. It was used to review Greptile review comment; I've amended it locally to drop the trailer and force-pushed, so the history is now clean |
Jem256
left a comment
Jem256
left a comment
There was a problem hiding this comment.
@chandachewe10 great work with the pr. i left a few minor comments.
| file.networks.forEach(network => { | ||
| const pre = `[${network.id}] ${network.name}:`; | ||
| network.nodes.bitcoin.forEach(node => { | ||
| // the zmqHashBlock port was added in PR #1020 to expose the hashblock stream to the host |
There was a problem hiding this comment.
nit: this comment is misleading because the pr number doesn't exist. I suggest removing the PR #1020 reference
| if (!node.ports.zmqHashBlock) { | ||
| debug(`${pre} set ZMQ Hash Block port for Bitcoin node ${node.name}`); | ||
| node.ports.zmqHashBlock = BasePorts.bitcoind.zmqHashBlock; | ||
| } |
There was a problem hiding this comment.
the greptile-apps comment here was not resolved.
|
@Jem256 thanks will work on the comments and notify you once done |
2 participants
Fixes #1020 (upstream jamaljsr/polar)
The bitcoind daemon publishes three ZMQ streams:
Previously only rawblock and rawtx were mapped to the host, making the hashblock stream inaccessible from outside Docker. Additionally the ConnectTab labels were ambiguous ('ZMQ Block Host'), not distinguishing rawblock from hashblock.
Changes:
Closes #1020