Do not round up SV1 difficulties#2227
Conversation
…e is lost in the process.
Extract the power-of-two rounding into advertised_sv1_difficulty_from_sv2_target() so translation proxies can reconstruct the exact difficulty a downstream miner was sent: they must validate downstream shares against the advertised difficulty while filtering upstream submissions with the exact upstream target, and the rounding policy needs a single source of truth for the two to agree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitGab19
left a comment
GitGab19
left a comment
There was a problem hiding this comment.
ACK on changing the rounding behavior, but I see a couple of improvements to be made before proceeding with this PR.
|
If there is no particular reason for rounding to a power of 2, then the target should just be exposed as is. Should I close this and the companion PR and just offer that as a simple PR? |
|
The reason behind the rounding is described under this past issue: stratum-mining/sv2-apps#561 From my side your PR here is good enough, as it rounds down (and not up) diffs, which is the major problem of the current implementation. It also provides a way for tProxy to validate shares with the advertised diffs, which is also the correct thing to do. I would simply modify the two things I pointed out above, but if you prefer I can push a commit on top of yours directly on this PR, or give you a commit to cherry-pick. |
|
Easiest and fastest would be for you to push a commit on top I suspect. Thanks for review. |
Add a helper that returns the Target corresponding to the SV1 difficulty advertised by the integer power-of-two rounding policy. Also reject rounding thresholds below 1.0 so the policy cannot round sub-1 difficulties up to 1.
Patch bump for the SV1 difficulty rounding fix and advertised target helper.
|
@ckolivas I just pushed two commits on top of yours. Do you want to update the PR on |
|
Please just wrap this up :) |
2 participants
Rounding up difficulty to a power of 2 compared to the upstream pool guarantees hashrate loss if the pool provides non power of 2 difficulties. If power of 2 is desired for some compatibility reason, you should round down instead of up to not lose hashrate. This will however lead to shares being rejected that are above the power of 2 difficulty but below the pool's difficulty. Whether a filter should be applied for shares higher than the power of 2 difficulty but below the pool's difficulty, or just pass the pool's actual difficulty instead is for debate. I'd be inclined to simply pass the pool's difficulty as is instead. I have in addition added the necessary code to do the filtering in mining-apps but as rust is not my speciality it would require co-authorship from claude and I'm unsure of the current policy wrt AI code here. The AI second patch and the following pull request stratum-mining/sv2-apps#630 are dependent on this one.