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

Lowdiff shares (2nd update #2718) #2728

Merged
merged 4 commits into from
May 12, 2018
Merged

Conversation

r4sas
Copy link
Contributor

@r4sas r4sas commented May 11, 2018

Please verify and test before merging.

@r4sas r4sas changed the title Lowdiff shares (2nd update) Lowdiff shares (2nd update #2718) May 11, 2018
Copy link
Contributor

@TheSerapher TheSerapher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check my change comment, other than that it looks okay. Make sure to test it a bit

IFNULL(SUM(difficulty) / ?, 0) AS sharerate,
IFNULL(SUM(difficulty), 0) AS shares,
IFNULL(AVG(difficulty), 0) AS avgsharediff
IFNULL(SUM(difficulty) / ?, " . $this->coin->getShareDifficultyPrecision() . ") AS sharerate,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should remain 0, if the column is NULL assume 0. No rounding done here. Same for the others below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will change back

@TheSerapher
Copy link
Contributor

Once merged maybe @HashUnlimited can update their branch and re-run the DB schema migration. Don't forget to reset the DB_VERSION string again so it will run properly. Blocks should then show the proper amount of shares even with blocks < 1 diff share.

@HashUnlimited
Copy link

just resetting wallet and db again... moving @r4sas remote

@TheSerapher
Copy link
Contributor

@HashUnlimited @r4sas since you are both kinda doing some P2P coding and testing here, please let me know when this is ready for merge into development. I think it's close to ready if not completed.

I have looked into the float/double for shares and I am not entirely sure ... I doubt think that double precision on share difficulties per user (which is what it is in that table) would not exceed that many shares. But if we want to play save we could increase it to double (but potentially waste 4 bytes per entry since it's not used).

No idea what's best? What do you think?

@r4sas
Copy link
Contributor Author

r4sas commented May 12, 2018

In that case we can set to unsigned float because here will never be negative values.

@TheSerapher
Copy link
Contributor

Good compromise :) Doubles the range and should be sufficient for single-account share counting.

@HashUnlimited
Copy link

sufficient for sure, the maximum diff in all bitcoin based code is 2.6959947e+67

if we are confident we aren't running into

Using FLOAT might give you some unexpected problems because all calculations in MySQL are done with double precision.

I'm fine

@HashUnlimited
Copy link

one more tiny tiny comment once we are on it already, the Totals in the block statistics are showing integers, if it's simple it could be aligned to the new global style.

@r4sas
Copy link
Contributor Author

r4sas commented May 12, 2018

Totals in the block statistics are showing integers

Let me check...

@TheSerapher
Copy link
Contributor

Let's merge this now, looks complete and can be further tested on development. If more changes are required, we can start new PRs.

@TheSerapher TheSerapher merged commit d1d4a07 into MPOS:development May 12, 2018
@r4sas
Copy link
Contributor Author

r4sas commented May 12, 2018

I hope that's all :D

@HashUnlimited
Copy link

agreed, clicking through there's only one place left which is the graph but we don't want to be over-accurate here. for my part, all ready to test it squeeze it and try to break it ;-)

@r4sas
Copy link
Contributor Author

r4sas commented May 12, 2018

In Statitics -> Graphs -> sharerate?

@HashUnlimited
Copy link

exactly, that's the very last place I could find :D

@r4sas
Copy link
Contributor Author

r4sas commented May 12, 2018

Found where it happens. By the way I have opinion about that graphics:

data: {$YOURMININGSTATS nofilter},

We store that json string triple times. I think better to save it in one variable and reuse from it rather then store 3 times.
var hashChart = Morris.Line({
element: 'hashrate-area-chart',
data: {$YOURMININGSTATS nofilter},
xkey: 'time',
ykeys: ['hashrate'],
labels: ['Hashrate'],
pointSize: 1,
hideHover: 'auto',
resize: true,
fillOpacity: 1.00,
lineColors: ['#24A665'],
pointFillColors: ['#24A665'],
pointStrokeColors: ['#24A665']
});
var workersChart = Morris.Line({
element: 'workers-area-chart',
data: {$YOURMININGSTATS nofilter},
xkey: 'time',
ykeys: ['workers'],
labels: ['Workers'],
pointSize: 1,
hideHover: 'auto',
resize: true,
fillOpacity: 1.00,
lineColors: ['#24A665'],
pointFillColors: ['#24A665'],
pointStrokeColors: ['#24A665']
});
var shareCharts= Morris.Line({
element: 'sharerate-area-chart',
data: {$YOURMININGSTATS nofilter},
xkey: 'time',
ykeys: ['sharerate'],
labels: ['Sharerate'],
pointSize: 1,
hideHover: 'auto',
resize: true,
fillOpacity: 1.00,
lineColors: ['#24A665'],
pointFillColors: ['#24A665'],
pointStrokeColors: ['#24A665']
});

@TheSerapher
Copy link
Contributor

Probably a good idea ... If you find a quick and easy way to do it, go ahead :)

@MPOS MPOS deleted a comment from cudarender May 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants