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

fix stratum parsing not always counting rejected shares #163

Conversation

MoellerDi
Copy link
Contributor

I noticed that IMHO the code was not always counting rejected shares correctly; at least when using public-pool.io.
The UI was always reporting 0 rejected shares even when shares got rejected (as per the log)

I noticed public-pool is responding to an accepted share with something like:
{"id":803,"error":null,"result":true}

and a response to a rejected share is something like:
{"id":873,"result":null,"error":[21,"Job not found",""]}

Reading the code, it seems it was expected to get "result":false in case of rejected shares. However as per https://braiins.com/stratum-v1/docs is seems be normal for the protocol to response with "result":null instead of "result":false for an error.

This PR change the code to simply increase the counter of rejected shares whenever the value of "error:" is not null. It also add some logging for some unhandled stratum messages (like unhandled methods and result parsing).

In order to not confuse users with high numbers of rejected shares it might be a good idea to show just "x percent rejected" instead of the actual number.

@WantClue WantClue added the enhancement New feature or request label Apr 24, 2024
@skot
Copy link
Owner

skot commented Apr 29, 2024

this looks good to me! I think we should pull into the next esp-miner release

@benjamin-wilson benjamin-wilson merged commit 1da7132 into skot:master May 24, 2024
1 check passed
aaron3481 pushed a commit to aaron3481/ESP-Miner that referenced this pull request Jun 11, 2024
* fix stratum parsing not counting rejected messages

* add unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants