fix share difficulty validation - compare stratum server minimum_share_difficulty as unscaled difficulty

[bladedoyle]

Fix for compare of scaled and unscaled difficulty.

[phyro]

@quentinlesceller I suspect you might be appropriate for the stratum changes, but I can't assign you 👀

[tromp]

Instead of all those changes, isn't it simpler to scale up minimum_share_difficulty by the minimum graph weight (of C32) ?
That has the same behaviour, and doesn't require computing scaled_share_difficulty and unscaled_share_difficulty side by side.

[bladedoyle]

I think we probably want matching scaled and unscaled methods since the rust project can be used by upstream devs as a library. From personal experience (rust stratum server links to grin library for POW methods) I am certain that people look for it and are confused when no method exists. If you already know the technical details about what scaling is and how to calculate it it seems like a trivial point. But if you dont know the details and the method does not exist that makes things much harder. I would like to leave the method if you dont object too strongly?

[tromp]

Ok, if you think it's useful. But I don't like the separate from_proof_unscaled. Can you just inline it into to_unscaled_difficulty ?

[bladedoyle]

Sure, let me take a look at your suggestion and adjust the PR. I hope to find time in the next few days.

[bladedoyle]

@tromp Can you take another look to make sure I understood your request correctly.

[tromp]

Looks ok, with the suggested change in u64 literal notation.
The release this is going into should make clear that this affects miners who have set a minimum share difficulty greater than the default 1.

[bladedoyle]

@tromp Ok. This config item was always intended to be unscaled value, so this is a bug fix. But I do agree that it should be mentioned in the release notes in case people were using the scaled value there.

@quentinlesceller Can you please give a second review and merge if you agree with the changes?

[phyro]

We seem to show PR titles in https://github.com/mimblewimble/grin/releases. Should we introduce at some point some form of a changelog? e.g. https://github.com/mitsuhiko/redis-rs/blob/master/CHANGELOG.md

[bladedoyle]

I updated the PR title to be a more descriptive changes log entry

[quentinlesceller]

Looking good to me 👍

[phyro]

@tromp you still have "requested changes" on this one. I'm not sure whether this blocks the merge or not, but I'm assuming it's a 👍 from you as well now?