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

Share difficulty incorrect in share result log #251

Closed
JayDDee opened this issue Feb 27, 2020 · 12 comments
Closed

Share difficulty incorrect in share result log #251

JayDDee opened this issue Feb 27, 2020 · 12 comments

Comments

@JayDDee
Copy link
Owner

JayDDee commented Feb 27, 2020

Since at least v3.11.0 the share dificulty and share ratio have been incorrect.

This was dscovered while researching the relationship and handling of 256 bit hash and target
values and their corresponding difficulties. It is quite convoluted with redundant conversions.

To start stratum provides a stratum diff which is used to calculate a share's target diff. The target
diff is converted to a 256 bit hash target.

Getwork provides a 256 bit hash target which is converted to a target diff.

The hash value and diff are redundant, essentially reciprocals of each other. This also applies
to the target.

Eliminating the redundancy would be ideal but not practical. However a default can be established
that minimizes the convversions.

There are some performance considerations when choosing the default including:

Testing the hash with integer arithmetic requires 8 64 bit compares in the worst case.
But in the most likely and also best case it requires only 1. A best case optimization is
very efficient.

Testing the diff with double precision floating point requires 1 comparison in all cases, but an
expensive one.

The hash test is in the innner hash loop, executed for every hash iteration, so it benefits
from an efficient first test. This alone may more than compensate for any redundancy in
less travelled code.

Needs more research.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 27, 2020

I found this about floating point comparisons

https://stackoverflow.com/questions/22279619/floating-point-number-comparison-trick-inline-assembly

The float test isn't the problem, but there is the overhead to convert the 256 bit hash to a
floating point double precision diff for every nonce.

It seems pretty clear that the 256 bit raw hash is the preferred default fomat in the hash loop.
That requires converting the target diff to a 256 bit target hash for every new job. Theoretically
it should only be necessary for a new block if the net diff has changed or of the stratum diff
has changed.

@JayDDee JayDDee added the bug label Feb 27, 2020
@JayDDee
Copy link
Owner Author

JayDDee commented Feb 27, 2020

A hybrid solution is being considered. Use the 256 bit hash in the hash loop to do the first test
to quiclkly eliminate the vast majority, then convert to diff for the final full test. There is already
precedent. Interleaved hash does an identical initial test on hashq3 before eextracting the full
hash for further testing.

The interface will need some work.

The initial test can be either 32 bits or 64 bits. 64 bits (hash_q3) is preferred but 32 bits (hash_d7)
are used when the hash is interleaved by 32 bits. It has no effect on the final result but it
reduces the resolution of the first test by small amount.

Other than ensuring the initial blockheader data is updated with the correct nonce value all
the other code is common to all algos.

This creates the opportunity to create a single function to test the hash and submit a
share that passes the test. The function could also convert the 256 bit hash to share diff
for later stats use. It could be preceded by an integer test to screen for the need to extract
interleaved hash or to decide whether to do further testing requiring a conversion to double.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 27, 2020

I'm currently testing a fix as described above. It needs to be propagated to all algos individually.
The next release is another planned test releae and will have a few algos converted to cover the
three situations: 64 or 32 bit integer pretest prior to optionally doing the full test, or no pretest and
doing the full test.

The next full release will have all currently active algos converted.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 27, 2020

My curiosity is now turning to suspicion regarding the conversion from bitstream to double precsion
and back. Now that the sharediff is being displayed "correctly" I see another issue. There are no
shares with a diff close to the target diff. They are all 2 or more orders of magnitude better.
One of them is wrong.

This doesn't seem to be related to the enhancements so they may be deployed in a test build
while the latest problem is still being investigated.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 28, 2020

The incorrect data seems to be the sharediff. Some blocks sloved were falsely reported
supporting the notion that the sharediff is orders of magnitude too high.

This doesn't affect the hash test or the effective hash rate, only a stats error.
It also shows that the problem is with target_to_diff which is only used for stats.

This may result in calculating the sharediff differently, using the raw 256 bit hash and target,
since they seem to be correct.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 28, 2020

I changed the sharediff calculation to use the 256 bit hash and target, the same hash as computed
and the same target it was tested against. It's better but still doesn't look right. There are still
no shares close to the target diff the it seems to be off by less than one order of magnitude instead
of 2 or 3.

The targetdiff is typicallly around 5e-8 but there are no shares below 1e-7. I would expect more
hashes at lower difficulty with a hard drop off at the targetdiff. But the bulk of the shares are in
the e-6 range with fewer at e-7 and none at e-8.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 28, 2020

The main issue seems to be fixed in v3.12.4.5 but there'sstill some follow up I want to do to
make sure the job diff from stratum is being correctly converted to a 256 bit hash target.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 28, 2020

Solo shares are being reported as "Accepted" instead of "BLOCK SOLVED" as they should be.
The reported share diff is way below the net diff but the block is accepted by the wallet.
This implies the target is actually lower than net dif would suggest. The target diff will be added
to the new block log for comparison.

Also the share ratio is missing the % for getwork. It gets displayed for stratum using the same
format spec. Seems odd.

@JayDDee
Copy link
Owner Author

JayDDee commented Feb 28, 2020

Solved the % problem by changing the definition to be a fraction instead of a percentage.

@JayDDee
Copy link
Owner Author

JayDDee commented Mar 1, 2020

Found a bug in share diff calculation, simple fix. Explaims a lot of things.

@JayDDee
Copy link
Owner Author

JayDDee commented Mar 1, 2020

cpuminer-opt-3.12.5 s released with a fix for share difficulty.

@JayDDee
Copy link
Owner Author

JayDDee commented Mar 3, 2020

Closing.

@JayDDee JayDDee closed this as completed Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant