Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Worker specific hashrate tracking extension (SOLUTION N. 1) #108

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

GitGab19
Copy link
Collaborator

After discussing with @TheBlueMatt and @Fi3 about the possible DoS vectors introduced by PR #107 (specifically here), I decided to open a PR to implement SOLUTION N.1 outlined in this doc.

This PR adds an extension to directly send a user_identity field inside a new SubmitIdentifiedSharesExtended. The user_identity field is limited to a maximum of 32 bytes-length to not increase too much the additional bandwidth consumption for extended shares submissions.

@TheBlueMatt
Copy link

I left a further comment on the previous thread, but I don't think we need to do this as an extension with new messages, and we need to be careful with extension proliferation or everyone is gonna need to support a billion message types with a million combinations of fields all for the same thing #107 (comment)

@GitGab19
Copy link
Collaborator Author

How would you do it tho? Initially I was thinking about using a new flag in the SetupConnection to tell server that client is going to send user_identity at the end of SubmitSharesExtended message. But this new flag should be handled by existing implementations I guess.

@Fi3
Copy link
Contributor

Fi3 commented Nov 15, 2024

I left a further comment on the previous thread, but I don't think we need to do this as an extension with new messages, and we need to be careful with extension proliferation or everyone is gonna need to support a billion message types with a million combinations of fields all for the same thing #107 (comment)

do you want to modify the mining protocol? We already released it, braiins already support 2 version demand already support 1 version. While I'm ok in changing other protocols only supported by demand I think that we should stop changing the mining protocol at least. Otherwise we end up in a situation where we need to support multiple version of sv2 every version is labelled with version 2 and it will be a lot harder then to support a bunch of well specified extensions.

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.

3 participants