Skip to content

Replace pool_pass_full_users/pool_pass_workers with pool_username_behaviour and add "strip_worker" option#180

Open
luke-jr wants to merge 15 commits intoOCEAN-xyz:masterfrom
luke-jr:full_users_without_workers
Open

Replace pool_pass_full_users/pool_pass_workers with pool_username_behaviour and add "strip_worker" option#180
luke-jr wants to merge 15 commits intoOCEAN-xyz:masterfrom
luke-jr:full_users_without_workers

Conversation

@luke-jr
Copy link
Copy Markdown
Contributor

@luke-jr luke-jr commented Apr 4, 2026

No description provided.

@luke-jr luke-jr added this to the v0.5.0 milestone Apr 4, 2026
@luke-jr luke-jr requested review from Copilot and wizkid057 April 4, 2026 20:55
@luke-jr luke-jr added enhancement New feature or request refactor Reorganizing code labels Apr 4, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_worker behaviour 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 toward pool_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.

Comment thread src/datum_conf.c Outdated
Comment thread src/datum_conf.c
Comment thread src/datum_conf.c
Comment thread src/datum_conf.c
Comment thread src/datum_api.c
Comment thread www/config.html
Comment thread src/datum_protocol.c
Comment thread src/datum_protocol.c Outdated
Comment on lines 1359 to 1360
i += datum_protocol_submit_username(username, 385, &datum_config, pow->username);
i++; // already 0 from snprintf
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing bug; addressing in #181

Comment thread src/datum_api.c Outdated
Without this, only 384 bytes get written for the username, but extra space is included in the message before the rest of the data
@luke-jr luke-jr force-pushed the full_users_without_workers branch from 7b97492 to b772e10 Compare April 5, 2026 03:29
@luke-jr luke-jr force-pushed the full_users_without_workers branch from b772e10 to 89b8142 Compare April 5, 2026 03:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/datum_api.c Outdated
@luke-jr luke-jr force-pushed the full_users_without_workers branch from 89b8142 to 9cd58f5 Compare April 7, 2026 02:40
@luke-jr
Copy link
Copy Markdown
Contributor Author

luke-jr commented Apr 7, 2026

Rebased on #181

@luke-jr luke-jr force-pushed the full_users_without_workers branch from 9cd58f5 to 1e33e50 Compare April 7, 2026 02:43
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/datum_conf.c
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) {
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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))) {

Copilot uses AI. Check for mistakes.
Comment thread src/datum_conf.c
Comment on lines +67 to +74
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)) {
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Comment thread src/datum_api.c
Comment on lines +1213 to +1215
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));
}
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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));

Copilot uses AI. Check for mistakes.
Comment thread doc/usernames.md
Comment on lines 41 to +46
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).
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request refactor Reorganizing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants