Replace pool_pass_full_users/pool_pass_workers with pool_username_behaviour and add "strip_worker" option#180
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new datum.pool_username_behaviour configuration option (including a new strip_worker mode) and refactors username submission logic to centralize handling and add tests.
Changes:
- Add
strip_workerbehaviour and expose it in the web configurator and docs. - Refactor POW submission username formatting into a shared helper with new protocol tests.
- Extend config parsing/help to support function-based config items (
DATUM_CONF_FUNC) and migrate towardpool_username_behaviour.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| www/config.html | Adds strip_worker option to “Send Miner Usernames To Pool” UI |
| src/datum_protocol.c | Refactors username formatting into datum_protocol_submit_username and uses it in POW submit |
| src/datum_protocol_tests.c | Adds unit tests for username formatting behaviours |
| src/datum_gateway.c | Hooks protocol tests into --test |
| src/datum_conf.h | Adds DATUM_CONF_FUNC and function-pointer support in config item struct |
| src/datum_conf.c | Adds pool_username_behaviour config parsing and deprecation handling |
| src/datum_api.c | Updates configurator variable fill + config-set logic for new behaviours |
| doc/usernames.md | Documents pool_username_behaviour and new strip_worker mode |
| doc/example_datum_gateway_config.json | Updates example config to use pool_username_behaviour |
| CMakeLists.txt | Adds src/datum_protocol_tests.c to build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| i += datum_protocol_submit_username(username, 385, &datum_config, pow->username); | ||
| i++; // already 0 from snprintf |
There was a problem hiding this comment.
datum_protocol_submit_username returns snprintf's would-have-been-written length, but the caller uses it to advance i into msg. If the formatted username is truncated (buffer is 385 bytes, but mining_pool_address + '.' + username can exceed that), i will advance past the actual written bytes and subsequent writes will go out of bounds / corrupt the message. Clamp the returned length to the actual bytes written (<= username_sz-1) or have datum_protocol_submit_username return the clamped length.
| i += datum_protocol_submit_username(username, 385, &datum_config, pow->username); | |
| i++; // already 0 from snprintf | |
| datum_protocol_submit_username(username, 385, &datum_config, pow->username); | |
| i += strnlen(username, 385); | |
| i++; // skip the terminating NUL written by snprintf |
Without this, only 384 bytes get written for the username, but extra space is included in the message before the rest of the data
7b97492 to
b772e10
Compare
b772e10 to
89b8142
Compare
…otocol_submit_username
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… with datum.pool_pass_full_users=true
…trip worker names
…me_behaviour and add "strip_worker" option Backward compatibility included
89b8142 to
9cd58f5
Compare
|
Rebased on #181 |
9cd58f5 to
1e33e50
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cat = json_object_get(config, "datum"); | ||
| if (json_is_object(cat)) { | ||
| item = json_object_get(cat, "pool_pass_full_users"); | ||
| if (item && (!json_is_false(item)) ? !(datum_config.datum_pool_pass_full_users && datum_config.datum_pool_pass_workers) : datum_config.datum_pool_pass_full_users) { |
There was a problem hiding this comment.
The conditional intended to warn only when the deprecated key datum.pool_pass_full_users is present appears to have ?: precedence issues: when the key is missing (item == NULL), the expression evaluates to datum_config.datum_pool_pass_full_users and can emit a deprecation warning even though the deprecated option wasn’t configured. Wrap the ternary in parentheses (or restructure) so the warning is gated by item being non-NULL.
| if (item && (!json_is_false(item)) ? !(datum_config.datum_pool_pass_full_users && datum_config.datum_pool_pass_workers) : datum_config.datum_pool_pass_full_users) { | |
| if (item && (((!json_is_false(item)) ? !(datum_config.datum_pool_pass_full_users && datum_config.datum_pool_pass_workers) : datum_config.datum_pool_pass_full_users))) { |
| if (json_is_string(j)) { | ||
| const char * const s = json_string_value(j); | ||
| switch (json_string_length(j)) { | ||
| case 4: | ||
| if (0 == strcasecmp("pass", s)) break; | ||
| return -1; | ||
| case 6: | ||
| if (0 == strcasecmp("worker", s)) { |
There was a problem hiding this comment.
datum_conf_username_behaviour uses strcasecmp, but this file doesn’t include <strings.h> (POSIX), which can cause build failures (especially with C23 where implicit declarations are not allowed). Add the proper include (or avoid strcasecmp).
| if (!datum_config.datum_pool_pass_full_users) { | ||
| datum_api_json_modify_new("datum", "pool_pass_workers", json_boolean(datum_config.datum_pool_pass_workers)); | ||
| } |
There was a problem hiding this comment.
When writing legacy keys for backward compatibility, the passthrough path updates pool_pass_full_users but may leave an existing pool_pass_workers key set to false (e.g. after switching from "private"). That makes the persisted config internally inconsistent and triggers the new startup deprecation warnings/UI state mismatches. Ensure pool_pass_workers is also set to true (or delete it) when pool_pass_full_users is true and you choose to keep legacy keys.
| if (!datum_config.datum_pool_pass_full_users) { | |
| datum_api_json_modify_new("datum", "pool_pass_workers", json_boolean(datum_config.datum_pool_pass_workers)); | |
| } | |
| datum_api_json_modify_new("datum", "pool_pass_workers", json_boolean(datum_config.datum_pool_pass_workers)); |
| There are three different ways to pass usernames to your pool. | ||
|
|
||
| By default, the Stratum username is always passed in full, as-is. | ||
| You can make this explicit by setting `datum`.`pool_pass_full_users` to `true` in the config file, or "Send Miner Usernames To Pool: Override Bitcoin Address" in the web configurator. | ||
| You can make this explicit by setting `datum`.`pool_username_behaviour` to `"passthrough"` in the config file, or "Send Miner Usernames To Pool: Override Bitcoin Address" in the web configurator. | ||
|
|
||
| If you change `datum`.`pool_pass_full_users` to `false`, you can then set `datum`.`pool_pass_workers` instead (or "Send Miner Usernames To Pool: Send as worker names" in the web configurator). | ||
| You can also set `datum`.`pool_username_behaviour` to `"worker"` (or "Send Miner Usernames To Pool: Send as worker names" in the web configurator). |
There was a problem hiding this comment.
The doc still says there are “three different ways” to pass usernames, but this PR adds a fourth (strip_worker). Update that sentence/count to match the newly documented behaviors.
No description provided.