Skip to content

Implement Extensions Negotiation and Worker Specific Hashrate Tracking in applications - #65

Merged
GitGab19 merged 8 commits into
stratum-mining:mainfrom
GitGab19:extensions-support-in-apps
Nov 25, 2025
Merged

Implement Extensions Negotiation and Worker Specific Hashrate Tracking in applications#65
GitGab19 merged 8 commits into
stratum-mining:mainfrom
GitGab19:extensions-support-in-apps

Conversation

@GitGab19

@GitGab19 GitGab19 commented Nov 4, 2025

Copy link
Copy Markdown
Member

This PR implements the two extensions on our applications.

It's based on my other work on stratum, which can be found here: stratum-mining/stratum#1983.

Still WIP:

  • Pool
  • tProxy
  • JDC
  • integration tests with extensions enabled

Comment thread miner-apps/jd-client/src/lib/downstream/mod.rs Outdated
Comment thread miner-apps/jd-client/src/lib/job_declarator/message_handler.rs Outdated
Comment thread miner-apps/jd-client/src/lib/job_declarator/mod.rs
Comment thread miner-apps/jd-client/src/lib/template_receiver/mod.rs
Comment thread miner-apps/jd-client/src/lib/template_receiver/message_handler.rs Outdated
Comment thread miner-apps/jd-client/src/lib/upstream/message_handler.rs Outdated
@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch from 2a98bea to 97503fc Compare November 10, 2025 13:04
@GitGab19
GitGab19 marked this pull request as ready for review November 10, 2025 13:04
@GitGab19

Copy link
Copy Markdown
Member Author

I still need to consider if it's worth adding some new integration tests, but the PR is definitely ready for review.

It's entirely based on my other work in stratum-mining/stratum#1983, so you should take a look at both PRs if you want to review it.

@bit-aloo

Copy link
Copy Markdown
Member

@GitGab19 Currently, our approach for handling roles is to convert the frame into a message as soon as it’s received. We avoid passing or circulating raw frames internally since we’ve previously observed occasional message corruptions with that approach. I’ll review this PR soon that should give me a clearer picture to discuss the frame orchestration in more detail. I am still reviewing the core extension stuff.

@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch 4 times, most recently from d1025f3 to 072d987 Compare November 11, 2025 13:47
@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch 3 times, most recently from 592ac6b to 3ce3a29 Compare November 19, 2025 23:22
Comment thread pool-apps/pool/src/lib/channel_manager/mining_message_handler.rs Outdated
@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch 3 times, most recently from 06fcc96 to b39fd82 Compare November 24, 2025 13:38
@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch from b39fd82 to 22601a0 Compare November 24, 2025 15:48

@xyephy xyephy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@GitGab19 Nice work on the extensions implementation! Everything compiles, tests pass, and all of Shourya's feedback is addressed.

Edge Case Found (not blocking, pre-existing from main):
Line 423 in channel_manager.rs can panic if downstream sends binary (non-UTF8) user_identity:

  • Binary input (255 bytes) → hex encoding (0x + 510 chars = 512 bytes)
  • Append .translator-proxy (+18 bytes) = 530 bytes total
  • .try_into::<Str0255>().unwrap() panics because 530 > 255 max

This is unrelated to your extensions work - just something I noticed during review. Happy to open a follow-up PR if you think it's worth fixing, otherwise we can leave it. What do you think?

@GitGab19

Copy link
Copy Markdown
Member Author

Line 423 in channel_manager.rs can panic if downstream sends binary (non-UTF8) user_identity

The user_identity used there comes from the internal OpenExtendedMiningChannel message which is crafted by the Sv1Server, which takes the user_identity value from the tProxy config file. Probably we should also add some checks there (?)

Happy to open a follow-up PR if you think it's worth fixing, otherwise we can leave it.

Let's log this as an issue, and let's tackle it in a follow-up PR.

@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch from 22601a0 to 25aa0ae Compare November 25, 2025 11:00
Comment thread pool-apps/pool/src/lib/channel_manager/mining_message_handler.rs Outdated
Comment thread pool-apps/pool/src/lib/channel_manager/mining_message_handler.rs
Comment thread miner-apps/translator/src/lib/sv2/upstream/upstream.rs Outdated
Comment thread miner-apps/translator/src/lib/sv2/channel_manager/channel_manager.rs Outdated
Comment thread miner-apps/translator/src/lib/error.rs Outdated
Comment thread miner-apps/jd-client/src/lib/upstream/mod.rs
@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch 2 times, most recently from 04c26de to f0f487a Compare November 25, 2025 15:19
Comment thread stratum-apps/Cargo.toml Outdated

@bit-aloo bit-aloo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK

@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch from f0f487a to c2b6d63 Compare November 25, 2025 15:48
@GitGab19
GitGab19 force-pushed the extensions-support-in-apps branch from c2b6d63 to 22709dc Compare November 25, 2025 15:56
@GitGab19
GitGab19 merged commit 5281fd3 into stratum-mining:main Nov 25, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants