yaml based configuration by jbride · Pull Request #36 · 256foundation/mujina

jbride · 2026-03-05T23:06:39Z

Details of implementation can be found here.

jayrmotta · 2026-03-10T12:14:26Z

Hey @jbride, good job with the pull request mate! I wish GitHub would allow me to submit a proper review, so I'll do it with comments for now.

Found an issue with the CPU miner config:

▪ MUJINA__DAEMON__LOG_LEVEL=debug \
MUJINA__API__LISTEN=0.0.0.0:7785 \
MUJINA__POOL__URL=stratum+tcp://public-pool.io:3333 \
MUJINA__POOL__USER=myuser \
MUJINA__POOL__PASSWORD=mypassword \
MUJINA__BACKPLANE__USB_ENABLED=false \
MUJINA__BOARDS__CPU_MINER__ENABLED=true \
MUJINA__BOARDS__CPU_MINER__THREADS=4 \
cargo run -p mujina-miner --bin mujina-minerd
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.24s
     Running `target/debug/mujina-minerd`
09:07:16 INFO  daemon: USB discovery disabled (backplane.usb_enabled = false)
09:07:16 INFO  daemon: CPU miner enabled
               threads=4, duty=50
09:07:16 INFO  daemon: Started.
09:07:16 INFO  daemon: For debugging, set RUST_LOG=mujina_miner=debug or trace.
09:07:16 INFO  backplane: CPU miner board connected.
               board=CPU Miner, threads=4, duty=50
09:07:16 INFO  job_source::stratum_v1: Waiting for hash threads before connecting
               pool=stratum+tcp://public-pool.io:3333
09:07:16 ERROR backplane: Failed to create CPU miner board
               board=CPU Miner, error=Configuration error: CPU miner not configured (MUJINA_CPU_MINER not set)
09:07:16 INFO  api::server: API server listening.
               url=http://0.0.0.0:7785
09:07:16 WARN  api::server: API server is bound to a non-localhost address (0.0.0.0). This exposes the API to the network without authentication.
^C09:07:21 INFO  daemon: Received SIGINT.
09:07:21 INFO  scheduler: Mining status.
               uptime=5s, hashrate=--, shares=0
09:07:21 INFO  daemon: Exiting.

I asked my LLM to provide a fix and then it provided this solution, maybe you want to port it to your branch.

jayrmotta · 2026-03-10T12:26:36Z

Another improvement would be replacing or removing references to the old environment variables. Found instances on the following files:

docs/api.md
mujina-miner/src/cpu_miner/config.rs
mujina-miner/src/cpu_miner/mod.rs
.github/DISCUSSION_TEMPLATE/issue-triage.yml
docs/cpu-mining.md
mujina-miner/src/bin/cli.rs
README.md
mujina-miner/src/bin/minerd.rs
mujina-miner/src/stratum_v1/client.rs
docs/container.md

Some of the mentions are in the context of explaining this PR's changes but some are just residue, if left like that it might confuse and misinform future developers.

jayrmotta · 2026-03-10T12:32:52Z

nit: Maybe we should find ways to avoid committing real config files by accident and leaking secrets? For one we could add mujina.yaml to .gitignore.

jayrmotta · 2026-03-19T18:32:37Z

Hey @jbride, following up to see if you will continue with this implementation? I think this would be really useful.

If you want to make me a contributor in your fork I can amend/add the fixes and improvements there as well.

rkuester · 2026-03-19T18:57:17Z

I realize it would be helpful if I took a look at this sooner rather than later. Thanks for your patience!

jbride · 2026-04-16T00:52:40Z

Sorry about the delay.
Thanks for the feedback.
I'll get on it.

fix(config): complete migration from legacy env vars to unified config system The CPU miner board factory was still reading MUJINA_CPUMINER_THREADS via from_env() rather than using config passed through the transport event, causing the board to fail when only the new MUJINA__* style vars were set. This commit also fixes the following: - Add mujina.yaml to .gitignore - Thread CpuDeviceInfo through VirtualBoardFactoryFn so the board factory receives its config from the transport event instead of legacy env vars - Migrate ForcedRateConfig::from_env() to pool.forced_rate config key - Remove dead CpuMinerConfig::from_env() and its tests - Add bootstrap variable validation in Config::load_with() to detect MUJINA__CONFIG__* and MUJINA__DEFAULT__* misuse with a clear error - Document bootstrap variables (MUJINA_CONFIG_FILE_PATH, MUJINA_DEFAULT_CONFIG_PATH) and their distinction from MUJINA__* keys - Update all docs and source to use MUJINA__* style; purge legacy var references from README, api.md, container.md, cpu-mining.md, issue triage template, and stratum_v1 integration test - Consolidate config_priority_tests.rs and cpu_miner_tests.rs into daemon_integration_tests.rs

jbride · 2026-04-16T19:34:36Z

Hey @jayrmotta .
Please take a look at the latest.
Thanks again for the review.
Now synced with latest in main branch.

jayrmotta

Thanks for the recent update @jbride and for working together on this!

I have tested once again and confirm the head eb6b6c9 works as the documentation suggests.

My comments are most about style and structure. When it comes to the overall design I'm not entirely convinced with merging properties from different config sources, I believe it might lead to unintended situations/configurations.

Last but not least, I would suggest cleaning up the commit history. Arguably you could have multiple commits, but right now I'd say they aren't following Mujina's guidelines to have atomic conventional commits only.

jayrmotta · 2026-04-21T17:20:55Z

+  # -- Bitaxe family (BM1370-based boards) --
+  # NOTE: These fields are parsed but not yet wired to board behavior.
+  # They are reserved for near-term implementation.
+  bitaxe:
+    # Emergency shutdown temperature in Celsius.
+    # The board halts hashing if any sensor reads above this value.
+    temp_limit_c: 85.0
+
+    # Fan speed bounds as a percentage of full speed.
+    # The thermal controller adjusts within this range.
+    fan_min_pct: 20
+    fan_max_pct: 100
+
+    # Maximum board power consumption in watts.
+    # Null disables the power cap (hardware default applies).
+    power_limit_w: ~


I would rather just skip this and then each author will implement support for their upcoming features.

Kept thinking about this and wanted to clarify, I'm coming from a place of implementing the thermal control for the bitaxes and not knowing for sure what should be exposed, if you check this config file it has many properties and each of them are arguably important for this implementation.

I believe the properties there might change a lot and so it's introduced already "out of sync" while both this and #33 are open at the same time. If anything I'd be more inclined exposing them all and giving our users the power of customizing everything, but having safe defaults.

jayrmotta · 2026-04-21T17:23:21Z

+# ------------------------------------------------------------
+# hash_thread — ASIC hash thread tuning
+# NOTE: These fields are parsed but not yet wired to board behavior.
+# They are reserved for near-term implementation.
+# ------------------------------------------------------------
+hash_thread:
+  # Difficulty target configured on-chip for share reporting.
+  # Lower values produce more frequent shares (useful for health
+  # monitoring); higher values reduce message volume on large
+  # installations. The scheduler still applies pool difficulty
+  # filtering before forwarding shares to the pool.
+  chip_target_difficulty: 256


nit: May be worth pointing out which PR, branch, discussion, etc, led you to this near-term implementation.

jayrmotta · 2026-04-21T17:27:33Z

+### 2.2. User-specified location
+
+Set `MUJINA_CONFIG_FILE_PATH` to an absolute path to load a second config file
+that supplements (and overrides) the default location:
+
+```sh
+MUJINA_CONFIG_FILE_PATH=/home/operator/mujina.yaml mujina-minerd
+```
+
+Keys present in the user-specified file take precedence over the same keys in
+`/etc/mujina/mujina.yaml`. Keys absent from the user file fall back to the
+default file, then to hard-coded defaults.
+
+An example config file is provided at `configs/mujina.example.yaml`.
+
+### 2.3. Bootstrap variables
+
+`MUJINA_CONFIG_FILE_PATH` and `MUJINA_DEFAULT_CONFIG_PATH` use single
+underscores and are **not** part of the `MUJINA__*` config-key system. They are
+bootstrap variables read by the loader before the config system is constructed,
+so the double-underscore naming convention does not apply to them.
+
+`MUJINA_DEFAULT_CONFIG_PATH` overrides the default system config path
+(`/etc/mujina/mujina.yaml`). It is primarily intended for testing, where
+writing to `/etc` requires root access:
+
+```sh
+MUJINA_DEFAULT_CONFIG_PATH=/tmp/mujina-test.yaml mujina-minerd
+```


nit: I find it a bit awkward that we are making the default separator a double underscore but this property remains single underscore. Perhaps we can find a way for it to also follow the same pattern or to drop it and support it as a CLI flag instead?

jayrmotta · 2026-04-21T17:35:24Z

+## 4. CLI Flags
+
+CLI flags override all other sources, including environment variables. They are
+intended for one-off overrides and testing, not permanent configuration.
+
+`mujina-minerd` accepts the following flags:
+
+```
+USAGE:
+    mujina-minerd [OPTIONS]
+
+OPTIONS:
+    -c, --config <PATH>         Config file path (overrides MUJINA_CONFIG_FILE_PATH)
+        --log-level <LEVEL>     Log level: error, warn, info, debug, trace [default: info]
+        --api-listen <ADDR>     API listen address [default: 127.0.0.1:7785]
+        --pool-url <URL>        Pool URL, e.g. stratum+tcp://pool.example.com:3333
+        --pool-user <USER>      Pool worker username
+        --pool-pass <PASS>      Pool worker password
+    -h, --help                  Print help
+    -V, --version               Print version
+```


nit: So the CLI flags are like an opinionated API since it doesn't support the same set of parameters nor does it reflect the same namespace as this pull request enables for the alternative config sources (env variables or YAML files.)

Why can't we do the same here? Should the other sources go for abitrary keys instead of enforcing/coupling external APIs to internal structures?

Probably this is more of a style preference, worth mentioning and see how it resonates, but wouldn't block the overall solution.

jayrmotta · 2026-04-21T17:48:50Z

+const DEFAULT_CONFIG_PATH_ENV_VAR: &str = "MUJINA_DEFAULT_CONFIG_PATH";
+const CONFIG_FILE_ENV_VAR: &str = "MUJINA_CONFIG_FILE_PATH";


Maybe these should be derived using the prefix and the separator, so it's not awkard having different patterns or prefixes if the users decide to customize.

jayrmotta · 2026-04-21T17:52:08Z

+#[derive(Debug, Clone, Deserialize, Serialize)]
+#[serde(default, deny_unknown_fields)]
+pub struct BitaxeConfig {
+    pub temp_limit_c: f32,
+    pub fan_min_pct: u8,
+    pub fan_max_pct: u8,
+    pub power_limit_w: Option<f32>,
+}

-    /// Power limits
-    pub power_limit: Option<f32>,
+impl Default for BitaxeConfig {
+    fn default() -> Self {
+        Self {
+            temp_limit_c: 85.0,
+            fan_min_pct: 20,
+            fan_max_pct: 100,
+            power_limit_w: None,
+        }
+    }
 }


This will probably be affected by #33, you could omit it here and if approved before/after the other one I'll make amends.

- Remove unimplemented BitaxeConfig and HashThreadConfig from config struct and example YAML; accepted keys should have effect - Drop MUJINA_CONFIG_FILE_PATH and MUJINA_DEFAULT_CONFIG_PATH bootstrap env vars; config file path is now CLI-only via --config - Replace named CLI flags (--pool-url, --log-level, etc.) with a generic --set key=value flag using the same dot-path namespace as the YAML config, giving full coverage without coupling CLI names to internal struct fields - Update integration tests, docs, and example YAML to match

jbride · 2026-04-22T16:04:01Z

@jayrmotta Sounds good. The following is a summary of changes with the latest commit:

Remove unimplemented BitaxeConfig and HashThreadConfig from config
struct and example YAML
Drop MUJINA_CONFIG_FILE_PATH and MUJINA_DEFAULT_CONFIG_PATH bootstrap
env vars; config file path is now CLI-only via --config
Replace named CLI flags (--pool-url, --log-level, etc.) with a
generic --set key=value flag using the same dot-path namespace as
the YAML config, giving full coverage without coupling CLI names
to internal struct fields
Update integration tests, docs, and example YAML to match

jbride · 2026-04-29T02:01:43Z

Hello @jayrmotta . Any other suggestions you may have before @rkuester reviews this PR ?

jayrmotta · 2026-05-02T21:01:13Z

Hey @jbride! Sorry it took me long to answer, been a bit hectic over here.

At this point I would also like a third person to run and review it. Looking forward to having this in the main branch.

jbride · 2026-05-03T00:18:20Z

Thanks @jayrmotta .
@rkuester : please review and provide feedback. thank you.

average-gary · 2026-05-18T17:55:10Z

From dev call: @rkuester to write up feedback so there are specifics on config design to discuss

rkuester · 2026-05-31T01:29:51Z

Thanks for all the work here, @jbride, and for your patience through the back-and-forth. Thanks @jayrmotta too for the careful testing and review along the way.

Digging into this PR on dev call #2 is what made me realize we should pin down what we actually want from configuration before reviewing an implementation line by line. That's the "write up config design specifics" item from the call. I've now done that, written up as MIP-0001, the first Mujina Improvement Proposal:

256foundation/mujina-mips#1

(Also introduced in an Ideas discussion, #57, for anyone following along there.)

A couple of things worth saying. The requirements are a draft for discussion, not a spec to go implement, and I'd much rather you and @jayrmotta poke holes in them first, so comments and line suggestions on that PR are very welcome, especially where they conflict with what you ran into building this. And since this PR is the reason the requirements exist and remains a valuable reference for them, I'd like to keep it open while we converge on them rather than do a line-by-line review right now.

Once we're agreed on the requirements, let's plan the implementation together. They reach beyond what this PR set out to do (runtime changes made through the API and persisted, live subscriptions to changes, etc.), so the realistic shape is an incremental path rather than one big change, and this PR is a strong starting point for that.

Please don't read any of this as the work being tossed. It's the opposite. My next step is walking the dev-call group through the requirements for review, and I'd like to have you both in that.

yaml based configuration

fd6b321

jbride added 3 commits April 16, 2026 10:57

Merge branch 'main' into config

c17ceac

fixing source code formatting issues

eb6b6c9

jayrmotta reviewed Apr 21, 2026

View reviewed changes

rkuester mentioned this pull request May 31, 2026

MIP-0001: Configuration and API 256foundation/mujina-mips#1

Open

		const DEFAULT_CONFIG_PATH_ENV_VAR: &str = "MUJINA_DEFAULT_CONFIG_PATH";
		const CONFIG_FILE_ENV_VAR: &str = "MUJINA_CONFIG_FILE_PATH";

Conversation

jbride commented Mar 5, 2026

Uh oh!

jayrmotta commented Mar 10, 2026

Uh oh!

jayrmotta commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jayrmotta commented Mar 10, 2026

Uh oh!

jayrmotta commented Mar 19, 2026

Uh oh!

rkuester commented Mar 19, 2026

Uh oh!

jbride commented Apr 16, 2026

Uh oh!

jbride commented Apr 16, 2026

Uh oh!

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

jayrmotta Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jayrmotta Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jayrmotta Apr 21, 2026 • 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!

jayrmotta Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jayrmotta Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jayrmotta Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jayrmotta Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jbride commented Apr 22, 2026

Uh oh!

jbride commented Apr 29, 2026

Uh oh!

jayrmotta commented May 2, 2026

Uh oh!

jbride commented May 3, 2026

Uh oh!

average-gary commented May 18, 2026

Uh oh!

rkuester commented May 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jayrmotta commented Mar 10, 2026 •

edited

Loading

jayrmotta left a comment •

edited

Loading

jayrmotta Apr 21, 2026 •

edited

Loading