Floor division in target calculaton #758
Labels
No Label
bug
build
dependencies
developers
documentation
duplicate
enhancement
formatting
invalid
legal
mobile
obsolete
packaging
performance
protocol
question
refactoring
regression
security
test
translation
usability
wontfix
No Milestone
No project
No Assignees
1 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Bitmessage/PyBitmessage-2024-11-30#758
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
PyBitmessage currently uses floor division for target calculation and checking:
https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_singleWorker.py#L773
https://github.com/Bitmessage/PyBitmessage/blob/master/src/shared.py#L343
See also: https://www.python.org/dev/peps/pep-0238/
Example:
You see, result slightly differs with true division.
Is it intentional behaviour? If it is, then should it be documented in the wiki POW article?
This is not the intended behaviour. PyBitmessage should be changed. I've added "from future import division" to shared.py and class_singleWorker.py and reviewed the places where division is used. I've tested it some and if it continues to behave just fine then I'll merge it into master.
Thank you very much for pointing out the issue.
@Atheros1 fractional part shouldn't matter? For the example above we eventually will compare
POW <= 297422525267.8102
and it's the same as comparingPOW <= 297422525267
, right?In fact the current behaviour is even handier for javascript implementations since JS lacks 64-bit numbers and JS big number libraries only provide floor division.
Although I've noticed that we can slightly rearrange calculations and result will still be the same as true division but without the final fractional part:
Also, I'm slightly worrying for existing Bitmessage implementations: probably some of them have already implemented current PyBitmessage behaviour and this change will break compatibility.
The fractional part shouldn't matter. These formulas are all calculating the target. The target ends up getting compared to the trialValue in proofofwork.py. trialValue is always an integer. The loop runs until trialValue is less than or equal to target. So for example, if target = 5, and trialValue = 4 then the loop will keep going. If target = 4.1 and trialValue = 4 then the loop will end. If we use floor division to calculate target then the loop will end. So it is consistent whether or not we use floor division.
Regarding existing Bitmessage implementations, yes it is possible that it will have an effect but the probability is extremely small. The difference between 297422593171 and 297422525267 is 67904. The probability that a completed POW lands within that range by coincidence is 67904 / 297422593171 = 2.28e-7. So I'm not worried about it.