feat: expose stratum mining stats via Owner API#3897
Conversation
wiesche89
left a comment
wiesche89
left a comment
There was a problem hiding this comment.
This is a nice and useful feature, and the endpoint is wired to the live StratumStats correctly. Before merging, I’d clarify whether the API should return only active workers, avoid masking a missing provider as disabled, and preserve the public node_apis signature.
| /// Whether the stratum server is enabled in config | ||
| pub is_enabled: bool, | ||
| /// Whether the stratum server is currently running | ||
| pub is_running: bool, |
There was a problem hiding this comment.
The source flag is set on startup but never cleared on listener failure or shutdown. Is “currently running” too strong for the API contract?
There was a problem hiding this comment.
A port conflict still leaves this as true after the listener exits. Could we set it after a successful bind, clear it on exit, and test the bind-failure case?
Add get_mining_status to the Owner JSON-RPC API so headless operators can query the same stratum/worker stats shown on the TUI mining tab. Wire live StratumStats into node_apis and add `grin client miningstatus`. Closes mimblewimble#3201
f406b85 to
60178eb
Compare
- Filter mining status worker_stats to currently connected workers (StratumStats retains disconnected workers for the node lifetime), and drop the now always-true is_connected field from WorkerInfo - Return an internal error from get_mining_status when no stats provider is wired, instead of masking it as disabled - Restore the public node_apis signature; add node_apis_with_mining_stats for provider wiring - Document is_running as set-once at startup - Trim get_mining_status doc comment to match neighboring methods
60178eb to
120e0cd
Compare
| blocks_found: stats.blocks_found, | ||
| network_hashrate: stats.network_hashrate, | ||
| minimum_share_difficulty: stats.minimum_share_difficulty, | ||
| // worker_stats retains every worker that ever connected; only expose |
There was a problem hiding this comment.
The response is bounded now, but each request still scans the full worker history under the stats lock. Could we keep a bounded active-worker view and derive num_workers from it too?
| /// Whether the stratum server is enabled in config | ||
| pub is_enabled: bool, | ||
| /// Whether the stratum server is currently running | ||
| pub is_running: bool, |
There was a problem hiding this comment.
A port conflict still leaves this as true after the listener exits. Could we set it after a successful bind, clear it on exit, and test the bind-failure case?
| pub worker_stats: Vec<WorkerInfo>, | ||
| } | ||
|
|
||
| impl Default for MiningStatus { |
There was a problem hiding this comment.
This has no caller besides its own test and duplicates StratumStats::default(). Could we remove the impl and test to avoid default drift?
| } | ||
|
|
||
| #[test] | ||
| fn get_mining_status_with_provider() { |
There was a problem hiding this comment.
Could we test this once through the generated JSON RPC handler? The current tests only call Owner directly, so the wire response is not covered.
| pub fn get_mining_status(&self) -> Result<MiningStatus, Error> { | ||
| match &self.mining_stats { | ||
| Some(provider) => Ok(provider()), | ||
| None => Err(Error::Internal( |
There was a problem hiding this comment.
This is fixed now. Could you also update the PR description? It still says a missing provider returns a disabled default.
|
I’ve written a prototype Grin miner in Rust https://github.com/wiesche89/rust-miner . It can be used to test the new Owner API method end to end. I tested four connected miners this way. The API reported num_workers: 4 with four worker entries, then returned num_workers: 0 and an empty worker list after disconnecting them. C25 is only intended as a lightweight connection test, valid testnet mining requires C32. |
2 participants
Summary
Adds an Owner API endpoint for stratum mining statistics so operators running headless (no TUI) can inspect connected workers and share stats — the same data shown on the TUI mining tab.
Closes #3201
Changes
get_mining_statusmethod on Owner JSON-RPC (/v2/owner)MiningStatus/WorkerInfoin the API crateStratumStatsinto the Owner API via a provider callbackgrin client miningstatusdoc/api/api.mdExample
{ "jsonrpc": "2.0", "method": "get_mining_status", "params": [], "id": 1 }Returns whether stratum is enabled/running, worker count, block height, network difficulty/hashrate, blocks found, and per-worker share stats. When stratum is disabled (or no provider is wired), returns a disabled default rather than an error.
Test plan
MiningStatusserde roundtrip, default disabled stateOwner::get_mining_statuswith/without providerStratumStats→MiningStatusconversioncargo check -p grin --bin gringrin client miningstatus