iframe-proxy

Shivrajsoni · 2026-01-04T13:07:48Z

RPC Server and Braidpool-cli implementation

Adds JSON-RPC server for the Braidpool node with endpoints for querying braid data, mining info, peer management, and Bitcoin RPC proxying.

Implemented Methods

Most RPC methods are implemented by reading from shared state (braid, peer manager, stratum connections) and returning the requested data. Methods like getbead, getbeadcount, gettips, getmininginfo, getpeerinfo etc. query the in-memory braid structure and return JSON responses. The bitcoinproxy method forwards requests directly to Bitcoin Core via HTTP JSON-RPC.

Methods Not Implemented in this PR

getminer get detailed statistics about a specific miner identified by connection_id/extraonce1.
committedtransactions Transactions committed in beads but not yet mined (might use cmempool IPC) See this
blockstagedtransactions Transactions in the blocktemplate we're currently mining (might use cmempool IPC) See this
stagetransaction Add a transaction to our staged list. Can take a txid (from bitcoind) or a full tx.
cmempool:<method> proxy IPC calls to cmempoold using @zaidmstrr's prototype https://github.com/zaidmstrr/bitcoin-rpc-multiprocess
Note that cmempool is fast-changing (we have to add and remove txs every time we see a new bead to create block templates), so this may be of limited usefulness.

Sansh2356 · 2026-01-05T04:27:03Z

Shivrajsoni · 2026-01-05T08:42:26Z

hi @Sansh2356 Thanks for the review!
I’ve applied rustfmt, fixed the failing rpc_server.rs tests by assigning distinct ports to each spawned server (keeping parallel test execution in mind), and removed the todo! implementations as suggested to be handled in follow-up sub-PRs.

The PR description has also been updated accordingly. Please let me know if anything else needs adjustment.

Sansh2356 · 2026-01-09T07:29:32Z

ut-ACK have gone through code looks fine, will test it asap.

zaidmstrr

Thanks for working on this. I reviewed this and found some noticeable points to look at. You can review, implement and further discuss the points below. I give the opinions based on the high-level overview of the PR. More technical go-through might be discussed later on review follow ups.

zaidmstrr · 2026-01-09T15:52:05Z

+
+#[derive(Debug, Subcommand)]
+#[command(rename_all = "PascalCase")]
+enum Commands {


The RPC names here are in PascalCase which is generally not good to have as a JSON-RPC method name. Change these to simple lower_snake_case.

Agreed this should be changed .

Updated the RPC method names to use lower_snake_case instead of PascalCase

You can also remove the underscore from the names. For example getparents getmininginfo etc. As these are easier to type.

Some of the RPC method names still contain an underscore with them.

zaidmstrr · 2026-01-09T16:02:35Z

+
+        let hash = node
+            .parse::<BeadHash>()
+            .map_err(|_| ErrorObjectOwned::owned(1, "Invalid bead hash format", None::<()>))?;


This error is not propagating to the end user CLI upon passing invalid hex, instead of this the CLI is returning Error: error decoding response body as a common message to all other invalid arguments. This behaviour is same in other RPC methods also.

Fixed by properly propagating JSON-RPC errors to the CLI using a structured RpcCallError enum, avoiding generic decode errors for cases like invalid hex across all RPC methods.

zaidmstrr · 2026-01-09T16:05:02Z

+    async fn get_bead(&self, bead_hash: String) -> Result<Bead, ErrorObjectOwned> {
        let hash = bead_hash
            .parse::<BeadHash>()
            .map_err(|_| ErrorObjectOwned::owned(1, "Invalid bead hash format", None::<()>))?;


Same here this is not propagating to the end user CLI upon passing invalid args.

zaidmstrr · 2026-01-09T16:16:49Z

+        Commands::GetBead { bead_hash } => ("getbead", json!([bead_hash])),
+        Commands::AddBead { bead_data } => ("addbead", json!([bead_data])),
+        Commands::GetBeadCount => ("getbeadcount", json!([])),
+        Commands::GetCohortCount => ("getcohortcount", json!([])),


This command should return int instead of String and this is not related to your change but I think you can change this behaviour in rpc_server.rs::get_cohort_count

zaidmstrr · 2026-01-09T16:21:55Z

+        Commands::GetBeadCount => ("getbeadcount", json!([])),
+        Commands::GetCohortCount => ("getcohortcount", json!([])),
+        Commands::GetTips => ("gettips", json!([])),
+        Commands::GetCohort { cohort_id } => ("getcohort", json!([cohort_id])),


The name GetCohort is confusing this should be getcohortbyid or similar.

zaidmstrr · 2026-01-09T16:29:10Z

+        Commands::GetMiningInfo => ("getmininginfo", json!([])),
+        Commands::GetParents { bead_hash } => ("getparents", json!([bead_hash])),
+        Commands::GetChildren { bead_hash } => ("getchildren", json!([bead_hash])),
+        Commands::GetHwPath { limit } => ("gethwpath", json!([limit])),


This name doesn't seem clear GetHwPath. Try to make it more clear or use full spelling, consider using GetHighestWorkPathByCount or similar.

zaidmstrr · 2026-01-09T16:31:09Z

+        Ok(children_hashes)
+    }
+
+    async fn get_hwpath(&self, limit: u8) -> Result<String, ErrorObjectOwned> {


Currently typing the value greater than the available beads doesn't show some error or message to the user; it just shows all the hashes. Consider showing some kind of message to the user, like "The max available count is x" or "The provided count seems too high and does not exist" plus add some more info for the user so they can figure out what to do next.

Updated to handle this edge case.

When the requested count exceeds the available beads, the RPC now returns a clear error, and the CLI surfaces a user-friendly message (e.g., indicating the maximum available count and why the request failed). This helps users understand what went wrong and how to adjust their input.

zaidmstrr · 2026-01-09T16:33:25Z

+        }
+    }
+
+    async fn get_braid_info(&self) -> Result<Value, ErrorObjectOwned> {


GetBraidInfo showing me orphan_count 5920, which doesn't seem a true value here, but maybe this bug is not related to this code and part of the other file. But please debug and try to find out.

This is related to #299 and #309 should be solved after these patches are merged .

zaidmstrr · 2026-01-09T16:37:02Z

+    GetBraidInfo,
+
+    /// Get node information (libp2p PeerID, payout address, miner pubkey, etc.)
+    GetNodeInfo {


GetNodeInfo is very unclear in passing arguments; it doesn't state what I need to pass along with this method. Please clarify clearly to user what they need to pass with this RPC method.

Added documentation and clearer argument naming, specifically bead_hash, with inline comments explaining that it identifies which bead’s creator node information is being requested. This should make the expected input and usage explicit to users.

zaidmstrr · 2026-01-09T16:39:57Z

+    /// Get statistics about the IPC connection
+    GetIpcStats,
+
+    /// Get braid information (similar to getblockchaininfo in bitcoin-cli)


Remove similar to getblockchaininfo in bitcoin-cli from here it's unnecessary. You can replace this with some other information that this RPC can return.

Sansh2356

Small nits required kindly check @Shivrajsoni .

Shivrajsoni · 2026-01-12T12:10:40Z

hi @Sansh2356 @zaidmstrr Thanks for the review!
I have applied the requested changes

Copilot

Pull request overview

This PR introduces JSON-RPC server functionality and a command-line interface (braidpool-cli) for interacting with Braidpool nodes. The implementation adds comprehensive RPC endpoints for querying braid data, mining statistics, peer management, and transaction staging, while also providing a proxy mechanism for Bitcoin Core RPC calls.

Changes:

Added 15+ RPC methods including getbead, getmininginfo, getpeerinfo, gethighestworkpathbycount, and bitcoinproxy
Created new braidpool-cli package with command-line interface for all RPC endpoints
Refactored PeerManager and connection mapping visibility to support RPC server queries

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
node/src/rpc_server.rs	Implements RPC server with 15+ methods, Bitcoin RPC proxy, and comprehensive test coverage
node/src/main.rs	Integrates RPC server initialization with shared state (braid, peer manager, stratum connections)
node/src/peer_manager/mod.rs	Adds `get_peers_json()` method and makes `ip_addr` public for RPC access
node/src/stratum.rs	Makes `downstream_channel_mapping` public for RPC server access to miner information
node/src/ipc.rs	Adds RPC command channel handling for IPC statistics and transaction removal
node/src/cli.rs	Changes `rpcuser`/`rpcpass` from `Option<String>` to `String` with defaults
braidpool-cli/src/main.rs	New CLI tool with command parsing and RPC client implementation
braidpool-cli/Cargo.toml	New package dependencies for CLI tool
node/Cargo.toml	Adds `reqwest` dependency for Bitcoin RPC proxy
Cargo.toml	Adds `braidpool-cli` to workspace members

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T18:03:36Z

    /// Use this password for bitcoin RPC
-    #[arg(long, default_value = "")]
-    pub rpcpass: Option<String>,
+    #[arg(long, default_value = "pass")]


Using "pass" as the default RPC password is a critical security vulnerability. This weak default password combined with the "root" username creates an easily exploitable attack vector. Remove the default value and require users to explicitly set secure credentials.

Suggested change

#[arg(long, default_value = "pass")]

#[arg(long)]

Copilot · 2026-01-15T18:03:36Z

+    let rpc_proxy_tx_for_rpc = rpc_proxy_tx;
+    let rpc_proxy_rx_for_ipc = rpc_proxy_rx;


These intermediate variable names add unnecessary verbosity. The original rpc_proxy_tx and rpc_proxy_rx names are already descriptive. Consider removing these intermediate assignments and using the original names directly where they're needed.

Sansh2356

t-ACK @Shivrajsoni you can checkout the co-pilot review if you find any useful otherwise looks good to me .

zaidmstrr

Thanks for implementing and fixing the previously marked issues. Most of the code structure seems solid but there are some more potential issues I found related to performance, structure and user interface.

zaidmstrr · 2026-01-27T15:34:04Z

+    let (method, params) = match &cli.commands {
+        Commands::GetBead { bead_hash } => ("getbead", json!([bead_hash])),
+        Commands::AddBead { bead_data } => ("addbead", json!([bead_data])),
+        Commands::GetBeadCount => ("getbeadcount", json!([])),


It will be good if this Commands::GetBeadCount return an int instead of String, just do the same thing you did with Commands::GetCohortCount

fixed it , GetBeadCount and GetCohortCount returns int now instead of string

zaidmstrr · 2026-01-27T15:41:44Z

+        Ok(children_hashes)
+    }
+
+    async fn get_highest_work_path_by_count(&self, limit: u8) -> Result<String, ErrorObjectOwned> {


There are some inconsistent Stringly Typed return types which are creating some structural problems while delivering the response to the user. Currently a client needs to manually double-decode the JSON string inside the JSON response as a result, it shows the output like this:

./target/debug/braidpool-cli get_highest_work_path_by_count 10 "[\"000000000b3ad6bf987cf52981945c72a3c9db445d224d3eb87449ed700262d0\",\"00000000035ee15c78a7a2a588f5ce3df1ed081d83811831d934643b602c2555\",\"000000004d6ef66c16eaa64a297d3dbd91bf21574bb60919f8076bb42c44e913\",\"00000000d4dd3010388ea45036a3a16aaf3831136e918716e469a5d7b31b5d2c\",\"00000000c7368ce0bd30f56dc296750c451e433606eb74d2a1708c9f30130e73\",\"00000000405833a00c55acd14e60009e8a23e698b713abc034395e152a8df293\",\"000000006cb457f27806c757f449aa7d24c2f4d7411718b4c7fa89fc5ca08a9a\",\"00000000f647585406045342e7c23789a7d7f962676033f7441d7b4631ba659b\",\"000000003459c46e416cd4d87adeff3c20de1ce4b92cfc393f48057d9ddd93e8\",\"000000000080b772d9a49dc7f404ea0aba5aae1a99422fc60b2f53f649526cf2\"]"

while the desired way of showing is like this:

./target/debug/braidpool-cli get_highest_work_path_by_count 10 [ "000000000b3ad6bf987cf52981945c72a3c9db445d224d3eb87449ed700262d0", "00000000035ee15c78a7a2a588f5ce3df1ed081d83811831d934643b602c2555", "000000004d6ef66c16eaa64a297d3dbd91bf21574bb60919f8076bb42c44e913", "00000000d4dd3010388ea45036a3a16aaf3831136e918716e469a5d7b31b5d2c", "00000000c7368ce0bd30f56dc296750c451e433606eb74d2a1708c9f30130e73", "00000000405833a00c55acd14e60009e8a23e698b713abc034395e152a8df293", "000000006cb457f27806c757f449aa7d24c2f4d7411718b4c7fa89fc5ca08a9a", "00000000f647585406045342e7c23789a7d7f962676033f7441d7b4631ba659b", "000000003459c46e416cd4d87adeff3c20de1ce4b92cfc393f48057d9ddd93e8", "000000000080b772d9a49dc7f404ea0aba5aae1a99422fc60b2f53f649526cf2" ]

fixed the inconsistent stringly typed return types by updating the CLI to stop doing the extra decode and to pretty-print the response. That should give the formatted list you described.

zaidmstrr · 2026-01-27T15:44:24Z

+        Ok(count as u64)
+    }
+
+    async fn get_cohort_by_id(&self, cohort_id: u64) -> Result<String, ErrorObjectOwned> {


This function also has the inconsistent Stringly Typed return types problem. The current behaviour is like this:

./target/debug/braidpool-cli get_cohort_by_id 10 "[\"000000002ae201f1836157be9d9334d57f9264bfde07ee4cb5a27ae9564153f2\"]"

zaidmstrr · 2026-01-27T15:46:45Z

+            .map_err(|e| ErrorObjectOwned::owned(2, format!("Internal Error: {}", e), None::<()>))
+    }
+
+    async fn get_parents(&self, bead_hash: String) -> Result<String, ErrorObjectOwned> {


This function also has the problem of inconsistent Stringly Typed return types. The current behaviour is like this:

./target/debug/braidpool-cli get_parents 000000003459c46e416cd4d8 7adeff3c20de1ce4b92cfc393f48057d9ddd93e8 "[\"00000000f647585406045342e7c23789a7d7f962676033f7441d7b4631ba659b\"]"

zaidmstrr · 2026-01-27T18:06:28Z

+
+#[derive(Debug, Subcommand)]
+#[command(rename_all = "PascalCase")]
+enum Commands {


You can also remove the underscore from the names. For example getparents getmininginfo etc. As these are easier to type.

zaidmstrr · 2026-01-27T18:09:15Z

+    };
+
+    // Build HTTP client with timeout
+    let client = reqwest::Client::builder()


In bitcoin_proxy and call_bitcoin_rpc_direct, you are creating a new HTTP client for every single request which is not optimal, try to create a client a single time and reuse that for the rest of the session.

created a bitcoin_proxy http client once at service/session started ,stored it in BitcoinRpcConfig , and reused it , also done same for braidpool_cli http_client , also added timeout of 30 sec

zaidmstrr

Thanks for fixing most of the suggested things. But I saw a change which is partially implemented, you can take a look and fix that.

zaidmstrr · 2026-01-28T07:46:15Z

+
+#[derive(Debug, Subcommand)]
+#[command(rename_all = "PascalCase")]
+enum Commands {


Some of the RPC method names still contain an underscore with them.

Shivrajsoni · 2026-01-28T08:05:41Z

thnx for the review @zaidmstrr , i have implemented the changes

zaidmstrr

I found few performance optimizations. It will be good if you look at them.

zaidmstrr · 2026-02-06T19:54:52Z

+        let mut geo_groups = HashSet::new();
+        let mut inbound_count = 0;
+
+        for info in &connected_peers_info {


You're iterating through connected_peers_info twice first to collect the stats and then to build the JSON list.

I've refactored get_peers_json to make a single pass over self.peers.values().filter(|p| p.connected). In this single loop, both the aggregate statistics (inbound count, total latency for average, geo groups) and the individual peer JSON objects are collected simultaneously.

zaidmstrr · 2026-02-06T19:57:46Z

+            }
+        }
+
+        if latency_count > 0 {


This is redundant and used here and below inside json avg_latency_ms.

removed the logic from json

zaidmstrr · 2026-02-06T20:21:55Z

+    pub fn get_peers_json(&self) -> serde_json::Value {
+        use serde_json::json;
+
+        let connected_peers_info: Vec<&PeerInfo> =


Instead of allotting this here can you use the direct self.peers.values()?

zaidmstrr

Overall looks good, but there are some small nits. Also please resolve the conflict and push again.

zaidmstrr · 2026-02-17T15:16:00Z

    //result in same peerID leading to OutgoingConnectionError

    // let keypair = identity::Keypair::generate_ed25519();
+


nit: redundant newline.

Thanks for the feedback! I’ve resolved the conflict and addressed the nits. Please take another look.

zaidmstrr · 2026-02-17T15:16:22Z

+    let (rpc_proxy_tx, rpc_proxy_rx) = tokio::sync::mpsc::unbounded_channel::<RpcProxyCommand>();
+
+    // peer_manager_arc is created above and shared between swarm and RPC server
+


nit: redundant newline.

fix:rustfmt and removing todos

format fix copilot requeste change fixing test

zaidmstrr

Code ACK 29f04fe

Thanks for considering the previous suggestions. This looks good to me, although I have suggested one future optimization which we can do, but that is not related to this PR. So you or anyone else can take a look at that.

zaidmstrr · 2026-03-06T18:34:08Z

+
+    async fn get_miner_info(&self) -> Result<Vec<String>, ErrorObjectOwned> {
+        info!("Get Miner Info Request Received");
+        let connection_map = self.stratum_connection_mapping.lock().await;


Here this is locking the main stratum connection mapping on each RPC call; this can block the critical tasks when someone uses this RPC and stratum tries to update the state on connection mapping, for e.g. adding the new miner, removing the disconnected miner or sending messages to miners. This can be more impactful when someone uses the getminerinfo RPC aggressively. The solution which might be here is to wrap the connection_mapping with RwLock. And I guess this change is out of scope for this PR, so I'm proceeding with this, but this can be a potential future optimization. I think using this lock on connection_mapping is also useful throughout the codebase in many places.

Thanks for the review @zaidmstrr , I’ll open a follow-up issue and work on it separately

* rpc:enhancement and braidpool-cli addition fix:rustfmt and removing todos * refactor cli names and improve error handling format fix copilot requeste change fixing test * rpc: typed returns, shared HTTP client and also bitcoin client * Optimize JSON output and peer stats calculation * removed redundant newlines and resolved merge conflict

Sansh2356 requested review from Sansh2356, mcelrath and zaidmstrr and removed request for Sansh2356 January 5, 2026 04:00

zaidmstrr requested changes Jan 9, 2026

View reviewed changes

Sansh2356 requested changes Jan 9, 2026

View reviewed changes

Comment thread braidpool-cli/src/main.rs Outdated

Comment thread braidpool-cli/src/main.rs Outdated

Comment thread braidpool-cli/src/main.rs Outdated

Comment thread node/src/rpc_server.rs Outdated

Comment thread node/src/rpc_server.rs

Shivrajsoni force-pushed the rpc/braidpool-cli branch from 165f753 to de5f994 Compare January 12, 2026 12:07

priyashuu requested a review from Copilot January 15, 2026 18:02

Copilot AI reviewed Jan 15, 2026

View reviewed changes

priyashuu assigned Shivrajsoni Jan 16, 2026

Shivrajsoni force-pushed the rpc/braidpool-cli branch from cbea495 to 7c4509a Compare January 16, 2026 17:10

Sansh2356 self-requested a review January 26, 2026 03:09

Sansh2356 previously approved these changes Jan 26, 2026

View reviewed changes

zaidmstrr requested changes Jan 27, 2026

View reviewed changes

Shivrajsoni dismissed Sansh2356’s stale review via 465b516 January 27, 2026 19:53

Shivrajsoni force-pushed the rpc/braidpool-cli branch from b71f4c3 to 465b516 Compare January 27, 2026 19:53

zaidmstrr reviewed Jan 28, 2026

View reviewed changes

Shivrajsoni force-pushed the rpc/braidpool-cli branch from 465b516 to bd6d9f6 Compare January 28, 2026 08:03

Sansh2356 force-pushed the rpc/braidpool-cli branch from bd6d9f6 to 9c1cf68 Compare January 28, 2026 09:53

zaidmstrr reviewed Feb 6, 2026

View reviewed changes

Shivrajsoni force-pushed the rpc/braidpool-cli branch from 9c1cf68 to c355b77 Compare February 7, 2026 14:39

zaidmstrr reviewed Feb 17, 2026

View reviewed changes

Shivrajsoni added 2 commits February 23, 2026 18:03

rpc:enhancement and braidpool-cli addition

913dbae

fix:rustfmt and removing todos

refactor cli names and improve error handling

2e995e9

format fix copilot requeste change fixing test

Shivrajsoni added 3 commits February 23, 2026 18:15

rpc: typed returns, shared HTTP client and also bitcoin client

a1123ee

Optimize JSON output and peer stats calculation

f070a96

removed redundant newlines and resolved merge conflict

29f04fe

Shivrajsoni force-pushed the rpc/braidpool-cli branch from c355b77 to 29f04fe Compare February 23, 2026 13:07

zaidmstrr approved these changes Mar 6, 2026

View reviewed changes

zaidmstrr merged commit 84d50ad into braidpool:dev Mar 6, 2026
9 checks passed

Shivrajsoni mentioned this pull request Mar 6, 2026

Reduce lock contention on stratum_connection_mapping by using RwLock #404

Closed

priyashuu mentioned this pull request Mar 20, 2026

Integrate Braidpool Peers UI into Dashboard #413

Open

		let rpc_proxy_tx_for_rpc = rpc_proxy_tx;
		let rpc_proxy_rx_for_ipc = rpc_proxy_rx;

		//result in same peerID leading to OutgoingConnectionError

		// let keypair = identity::Keypair::generate_ed25519();

		let (rpc_proxy_tx, rpc_proxy_rx) = tokio::sync::mpsc::unbounded_channel::<RpcProxyCommand>();

		// peer_manager_arc is created above and shared between swarm and RPC server

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

Shivrajsoni commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

RPC Server and Braidpool-cli implementation

Implemented Methods

Methods Not Implemented in this PR

Uh oh!

Sansh2356 commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Shivrajsoni commented Jan 5, 2026

Uh oh!

Sansh2356 commented Jan 9, 2026

Uh oh!

zaidmstrr 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!

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!

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!

Sansh2356 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Shivrajsoni commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Shivrajsoni commented Jan 4, 2026 •

edited

Loading

Sansh2356 commented Jan 5, 2026 •

edited

Loading