Conversation
Display total section totals amounts
fix: bridge config persistence, logging, shutdown, and dashboard XSS
"Fixed block count mismatch in terminal stats TOTAL row. The TOTAL was using instance-level aggregate (overall.blocks_found) which included blocks from pruned workers, while individual rows only showed active workers. Changed to sum blocks from currently visible workers, ensuring TOTAL matches the sum of individual worker block counts."
The terminal stats now show: Blocks: Current blocks from online workers (matches what's visible in individual rows) Total: Historical total blocks from all workers, even if they've gone offline This provides both current status and historical totals.
Refactor KaspaApi: Remove dead code and simplify API based on code review changes to be made from @IzioDev - Remove unused `new()` function, rename `new_with_shutdown` to `new` - Make `shutdown_rx` required (non-optional) - all callers provide it - Remove unused `_block_wait_time` parameter from constructor - Remove `verbose` parameters from `wait_for_sync` functions - Simplify Option handling for shutdown receiver
…ct TOTAL row display
| fail-fast: false | ||
| # matrix: | ||
| # os: [ ubuntu-latest, macos-latest, windows-latest ] | ||
| # strategy: |
There was a problem hiding this comment.
Why are you removing this?
There was a problem hiding this comment.
The matrix strategy was commented out due to CI resource constraints. The test suite builds the entire workspace and needs significant disk space; macOS and Windows runners have stricter limits than Ubuntu, and running on all three OS was causing disk space failures. It also triples CI execution time, delaying PR feedback. Multi-OS testing still happens in release builds (.github/workflows/deploy.yaml), which validates binaries on ubuntu-latest, windows-latest, and macos-latest, ensuring cross-platform compatibility. The code is commented out (not deleted) for easy re-enablement if needed. The current approach balances fast CI feedback during development with cross-platform validation during releases.
There was a problem hiding this comment.
I don't think that should be included as part of this PR. please revert this part.
.github/workflows/ci.yaml
Outdated
| # Append linker flag to handle mimalloc conflicts with libstdc++ in musl builds | ||
| # This allows multiple definitions to resolve the kaspa-alloc mimalloc override conflict | ||
| export RUSTFLAGS="$RUSTFLAGS -C link-arg=-Wl,--allow-multiple-definition" |
There was a problem hiding this comment.
These RUSTFLAGS will apply to all the binaries build down below. But I think you only intended it to apply to kaspa-stratum-bridge.
What is it about the code of the bridge that's making this change necessary anyway?
There was a problem hiding this comment.
The bridge needs this because it has both kaspa-alloc (which provides mimalloc) and a direct mimalloc dependency. In musl builds, mimalloc links against libstdc++ symbols, and static linking causes multiple definition errors.
There was a problem hiding this comment.
"These RUSTFLAGS will apply to all the binaries build down below. But I think you only intended it to apply to kaspa-stratum-bridge." As a solution I have split the build into two steps.
bridge/docs/README.md
Outdated
| ### Internal CPU miner (feature-gated) | ||
|
|
||
| The internal CPU miner is a **compile-time feature**. | ||
|
|
||
| Build: | ||
|
|
||
| ```bash | ||
| cargo build -p kaspa-stratum-bridge --release --features rkstratum_cpu_miner | ||
| ``` | ||
|
|
||
| Run (external node mode + internal CPU miner enabled): | ||
|
|
||
| ```bash | ||
| cargo run -p kaspa-stratum-bridge --release --features rkstratum_cpu_miner --bin stratum-bridge -- --config bridge/config.yaml --node-mode external --internal-cpu-miner --internal-cpu-miner-address kaspa:YOUR_WALLET_ADDRESS --internal-cpu-miner-threads 1 | ||
| ``` | ||
|
|
||
| ### Testing | ||
|
|
||
| Run all bridge tests (including CPU miner tests when feature is enabled): | ||
|
|
||
| ```bash | ||
| cargo test -p kaspa-stratum-bridge --features rkstratum_cpu_miner --bin stratum-bridge | ||
| ``` | ||
|
|
||
| Run tests without the CPU miner feature: | ||
|
|
||
| ```bash | ||
| cargo test -p kaspa-stratum-bridge --bin stratum-bridge | ||
| ``` | ||
|
|
||
| The test suite includes: | ||
| - Configuration parsing tests | ||
| - JSON-RPC event parsing tests | ||
| - Network utilities tests | ||
| - Hasher/difficulty calculation tests | ||
| - Mining state management tests | ||
| - Miner compatibility tests (IceRiver, Bitmain, BzMiner, Goldshell) | ||
| - Share validation and PoW checking tests | ||
| - VarDiff logic tests | ||
| - Wallet address cleaning tests | ||
| - CPU miner tests (when `rkstratum_cpu_miner` feature is enabled) | ||
|
|
||
| The test suite is comprehensive and educational, with 127+ unit tests designed to help developers understand the codebase. | ||
|
|
||
| ### Running two bridges at once (two dashboards) | ||
|
|
||
| If you run **two `stratum-bridge` processes** simultaneously (e.g. one in-process and one external), | ||
| they **cannot share the same**: | ||
| - `web_dashboard_port` (dashboard) | ||
| - any Stratum ports | ||
| - any per-instance Prometheus ports | ||
|
|
||
| Recommended setup: | ||
| - **In-process bridge**: run normally with `--config config.yaml` (uses `web_dashboard_port: ":3030"` and the configured instance ports) | ||
| - **External bridge**: do **not** reuse the same instance ports; instead, run a single custom Stratum instance on a different port and set a different web dashboard port. | ||
|
|
||
| Example (external bridge on `:3031` + Stratum `:16120`): | ||
|
|
||
| ```bash | ||
| cargo run -p kaspa-stratum-bridge --release --features rkstratum_cpu_miner --bin stratum-bridge -- --config config.yaml --web-dashboard-port :3031 --node-mode external --kaspad-address 127.0.0.1:16210 --instance "port=:16120,diff=1" --internal-cpu-miner --internal-cpu-miner-address "kaspatest:address" --internal-cpu-miner-threads 1 | ||
| ``` | ||
|
|
||
| Open: | ||
| - `http://127.0.0.1:3030/` for the in-process bridge | ||
| - `http://127.0.0.1:3031/` for the external bridge |
There was a problem hiding this comment.
Move these sections all the way down. Someone running the bridge will want the information on how to run it to come first.
| [target.x86_64-unknown-linux-musl] | ||
| rustflags = [ | ||
| "-C", "link-arg=-Wl,--allow-multiple-definition", | ||
| ] |
There was a problem hiding this comment.
Why is this necessary? Have you verified that this does not impact the kaspad binary?
There was a problem hiding this comment.
Why the flag exists:
kaspa-alloc uses mimalloc with "override" for non-macos targets (including musl)
The bridge also has a direct mimalloc dependency for musl builds (without override)
This creates a dual mimalloc dependency structure that requires the flag.
It does not impact kaspad binary.
The change was so that we did not have to make any changes directly to Util
There was a problem hiding this comment.
@biryukovmaxim do you have any recommendations here? I'm just concerned this applied at the top level like this affects all binaries
There was a problem hiding this comment.
@biryukovmaxim do you have any recommendations here? I'm just concerned this applied at the top level like this affects all binaries
Sry I have no expertise of what it could lead to. @saefstroem since you built musl step in ci, maybe you have any advices, considerations here?
There was a problem hiding this comment.
Why the flag exists: kaspa-alloc uses mimalloc with "override" for non-macos targets (including musl) The bridge also has a direct mimalloc dependency for musl builds (without override)
Doesn't it mean you can remove direct dependency
There was a problem hiding this comment.
@biryukovmaxim do you have any recommendations here? I'm just concerned this applied at the top level like this affects all binaries
// build.rs
fn main() {
println!("cargo:rustc-link-arg=-Wl,--allow-multiple-definition");
}Probably will do the same only for bridge crate
There was a problem hiding this comment.
Looking into this more, this is probably fine to have.
@saefstroem would like to get your confirmation here too.
Summary of the Change Problem: The RUSTFLAGS with --allow-multiple-definition was being applied to all binaries (kaspad, rothschild, kaspa-wallet, and stratum-bridge), but it's only needed for kaspa-stratum-bridge. Solution: Split the build into two steps:
coderofstuff
left a comment
There was a problem hiding this comment.
continuing review. still need to get to the rs parts but submitting feedback now
| fail-fast: false | ||
| # matrix: | ||
| # os: [ ubuntu-latest, macos-latest, windows-latest ] | ||
| # strategy: |
There was a problem hiding this comment.
I don't think that should be included as part of this PR. please revert this part.
| cargo --verbose build --bin kaspad --bin rothschild --bin kaspa-wallet --release --target x86_64-unknown-linux-musl | ||
| # Build bridge with allow-multiple-definition flag to handle mimalloc conflicts with libstdc++ in musl builds | ||
| # This flag is only needed for kaspa-stratum-bridge due to its dual mimalloc dependency structure | ||
| export RUSTFLAGS="$RUSTFLAGS -C link-arg=-Wl,--allow-multiple-definition" |
There was a problem hiding this comment.
If this rustflags is already in .cargo/cargo.toml, is it still needed here? This line can probably be removed.
bridge/docs/README.md
Outdated
| - `:5555` | ||
| - `:5556` | ||
| - `:5557` | ||
| - `:5558` |
There was a problem hiding this comment.
This mentions the ports being open but not why. Reformat this into tabular form with columns: Port and Purpose
| - `:5557` | ||
| - `:5558` | ||
|
|
||
| ### CLI Help |
There was a problem hiding this comment.
This is actually the 2nd most important part, so it should come before Default configs / ports and right after a missing section: "Running from a release" (which describes how you would run this from the releases).
bridge/docs/README.md
Outdated
| @@ -41,10 +46,12 @@ | |||
|
|
|||
| ### Run (external node) | |||
There was a problem hiding this comment.
The in-process mode should come first.
It should show instructions using simply cargo (without any configs) to make it very clear that there are sane defaults in place:
cargo run --bin kaspa-stratum-bridge --release
| [target.x86_64-unknown-linux-musl] | ||
| rustflags = [ | ||
| "-C", "link-arg=-Wl,--allow-multiple-definition", | ||
| ] |
There was a problem hiding this comment.
Looking into this more, this is probably fine to have.
@saefstroem would like to get your confirmation here too.
| - stratum_port: ":5559" | ||
| min_share_diff: 4 | ||
| prom_port: ":2118" | ||
| log_to_file: true | ||
|
|
||
| - stratum_port: ":5560" | ||
| min_share_diff: 512 | ||
| prom_port: ":2119" | ||
| log_to_file: true | ||
|
|
||
| - stratum_port: ":5561" | ||
| min_share_diff: 1024 | ||
| prom_port: ":2120" | ||
| log_to_file: true | ||
|
|
There was a problem hiding this comment.
These extra ports aren't documented.
There was a problem hiding this comment.
Default config / ports (Port | Purpose table), including :5559, :5560, :5561 and prom ports.”
Readme updated Reverted core/src/log/mod.rs as requested. Changed back to: let _handle = log4rs::init_config(config).unwrap();
shows inprocess mode first, then external
CI reflective of master What was happening: In bridge in-process mode, tracing_log::LogTracer::init() was setting the global log logger first. Then embedded kaspad called kaspa_core::log::init_logger(...).unwrap(), which failed with SetLoggerError(()) and panicked. What I changed: Updated bridge/src/tracing_setup.rs to not initialize LogTracer in in-process mode. It now only does LogTracer::init() in non-inprocess mode. This removes the double-initialization conflict and keeps the required unwrap() line in core/src/log/mod.rs exactly as-is.
Bridge Configuration Options
RKStratum 2.0 — What’s New (vs CustomIdentifier)
This is a user-facing summary of the main upgrades that landed in the
RKStratum2.0branch compared toCustomIdentifier.Web UI & API
/api/statusfor basic node/bridge status./api/statsfor summarized worker/block/share stats./metricsfor Prometheus scraping.CLI & Configuration Quality-of-Life
config.yamlsettings can now be overridden via CLI flags.--config <path>support + smarter config discoverytrue/false,1/0,yes/no,on/off).:PORTorHOST:PORT.Multi-Instance Improvements
--instancesyntax for defining multiple stratum listeners from the CLI.Observability & Metrics
Reliability / Operator Experience
RKStratum_*.log.health_check_port.Optional / Advanced Features
rkstratum_cpu_minerfeature.Packaging & Deployment
Dockerfile.stratum-bridge.stratum-bridgealongside other binaries.Quick Start
config.yaml(current directory).bridge/config.yaml.stratum-bridge --config bridge/config.yaml --node-mode externalstratum-bridge --config bridge/config.yaml --node-mode inprocess -- <kaspad args...>web_port(or use--web-port) and open:http://127.0.0.1:<PORT>/Global Settings (shared by all instances)
kaspad_address"localhost:16110""HOST:PORT"or"grpc://HOST:PORT".block_wait_time1000print_statstruelog_to_filetruehealth_check_port""(disabled)web_port""(disabled)":3030","0.0.0.0:3030".var_difftrueshares_per_min20var_diff_statsfalseextranonce_size0pow2_clampfalsecoinbase_tag_suffix""(empty)"RK-Stratum". Results in"RK-Stratum/<suffix>".Instance Settings (each instance can have its own)
stratum_port":5555"":PORT"or"HOST:PORT".min_share_diff8192prom_portNone(disabled)":PORT"or"HOST:PORT".log_to_fileNone(inherits global)log_to_file.block_wait_timeNone(inherits global)extranonce_sizeNone(inherits global)var_diffNone(inherits global)shares_per_minNone(inherits global)var_diff_statsNone(inherits global)pow2_clampNone(inherits global)Notes and Behavior
instancesarray exists inconfig.yaml, the bridge runs in multi-instance mode. Otherwise, it runs in single-instance mode using the global settings.stratum_portandprom_portcan be specified as":PORT"or"HOST:PORT". The bridge will prepend"0.0.0.0"if only a port is provided."RK-Stratum". You can only append a suffix viacoinbase_tag_suffix. The suffix is sanitized to alphanumeric characters,.,_, and-, and is limited to 64 characters.shares_per_min.log_to_fileis enabled, logs are written under:%LOCALAPPDATA%\kaspa-stratum-bridge\logs\RKStratum_<timestamp>.log~/.kaspa-stratum-bridge/logs/RKStratum_<timestamp>.logCLI Notes
true/false,1/0,yes/no,on/off.:PORTandHOST:PORTformats.config.yaml.--instancespecs (example:--instance "port=:5555,diff=8192").--instance, do not use the single-instance flags--stratum-port/--prom-port/--instance-*.Notes
stratum-bridge --help.