-
Notifications
You must be signed in to change notification settings - Fork 387
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
Bugfix: Cap stratum target at pdiff 1 to avoid overflow issues #73
Conversation
The rest of the code chops off the first 32 bits of the target, resulting in bdiff 0.9999847412109375 (approximately pdiff 1) being treated as the impractical zero target. Limiting the maximum target to pdiff1 fixes this.
Note: for some reason poclbm claims shares are being accepted when this problem happens (but never sends anything to the pool). That is another bug, but not one I care to look into at the moment. |
About your last comment - share is reported as accepted only if confirmation is received from server. There is unique ID for each share. I am thankful you are looking at problems in POCLBM. Can you please provide more information about the overflow issue? |
I can't confirm it myself, but numerous users on Eligius have reported poclbm and/or GUIMiner claiming shares are accepted, despite never having sent anything to the server. Maybe they're confusing something else for "accepted"? Since stratum works with share targets in terms of bdifficulty, and the pool measures shares in pdifficulty, we are sending mining.set_difficulty(0.9999847412109375); this results in a target of 0x0000000100000000000000000000000000000000000000000000000000000000 (to approximate the real target of 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff, which is impossible to accurately transmit using bdiff). However, poclbm is treating this target as zero, I presume because it is ignoring the first 32 bits. With this limit in place, poclbm seems to correctly send all shares to the server. |
It seems to me the root problem is pool measuring shares in pdiff. Then, you 'correct' this by sending fractional value to set_difficulty. Not only fractional, but less than 1 which is a non-valid difficulty in every aspect. Can you please change pool code to measure shares properly? I must admit that pdiff was perhaps unfortunately introduced by me some time ago and deeply regret this. But I believe we should no longer support this way of counting shares. There is one true minimum difficulty and it is as defined in bitcoind's source, namely bdiff. One final note. Because bdiff is 'harder' than pdiff, POCLBM should never be able to send invalid shares towards pdiff counting pool. The only problem would be the slightly lower performance, roughly 0.0016% lower than expected. |
Pdiff is the proper way to measure shares, and we have no plans to change that to satisfy broken protocols or miners. Bdiff 1 is merely an approximation of Pdiff 1 resulting from bitcoin's custom floating-point target; there is no reason to be using it outside of blocks. Discarding valid shares is a bug, even if you were to reduce it from 100% to 0.0016% - it is far better to submit invalid shares than discard valid ones, especially when we're talking about something like 0.0000000000000000000001%. |
So just to be clear: By closing this, your intention is to refuse to fix this poclbm bug? |
yes |
I'm going to point out that whatever GUIminer uses to listen to poclbm finding shares is broken when using pdiff1 stratum shares. GUIminer shows accepted shares (at the expected rate for a hashrate) but there is zero network activity. Merging this fix would seem appropriate, as it is quite a simple solution to the issue. |
The rest of the code chops off the first 32 bits of the target, resulting in bdiff 0.9999847412109375 (approximately pdiff 1) being treated as the impractical zero target.
Limiting the maximum target to pdiff1 fixes this.