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

Optimize hashing of internal sha256d miner. #871

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

barrystyle
Copy link
Contributor

Since the datahash is not likely to change much between blocks (other than
work refresh), we can calculate it once and reuse the value.

Average hashrate beforehand on 4core vps was 4mh, averages around 5.5-6mh with
blocks successfully solved on devnet.

@CaveSpectre11
Copy link
Collaborator

I need to run some tests to make sure there isn't changes to the hash during a run (doesn't look like it from code inspection). While I'm doing that check:

uint256 localDataHash = pblock->GetDataHash();

This function needs to change, since it's specific to Sha256D and isn't a generic GetDataHash().

@CaveSpectre11 CaveSpectre11 added the Tag: Waiting For Code Review Waiting for code review from a core developer label Nov 22, 2020
Comment on lines 56 to 58
uint256 CBlockHeader::GetDataHash() const
{
CSha256dDataInput input(*this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is specific to Sha256; change the GetDataHash() to be reflective of that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uint256 CBlockHeader::GetSha256dMidstate() ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure; that works. Use your discretion; but just as long as it is identifiable as a sha256 function [I figured GetSha256dDataHash(), but whatever makes sense I'm good with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

amended in 5650b36

@CaveSpectre11 CaveSpectre11 added the Bounty: Assigned This issue is being worked on by someone that will earn a bounty reward. label Nov 22, 2020
Copy link
Collaborator

@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

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

@CaveSpectre11
Copy link
Collaborator

@barrystyle just `rebase -i HEAD~2' and fixup the second one so that it's only one commit, and we'll send it on to QA.

Also, add a stealth address. We don't have a lot to offer, but we'll get you something for a bounty.

Since the datahash is not likely to change much between blocks (other than
work refresh), we can calculate it once and reuse the value.

Average hashrate beforehand on 4core vps was 4mh, averages around 5.5-6mh with
blocks successfully solved on devnet.
@barrystyle
Copy link
Contributor Author

Appreciated, but theres no need.. enjoy!

Copy link
Collaborator

@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

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

utACK b3603ed

Copy link
Collaborator

@CaveSpectre11 CaveSpectre11 left a comment

Choose a reason for hiding this comment

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

ACK b3603ed

@CaveSpectre11 CaveSpectre11 added Tag: Waiting For QA A pull review is waiting for QA to test the pull request and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels Nov 24, 2020
@CaveSpectre11 CaveSpectre11 added this to the v1.2.0 milestone Nov 24, 2020
@codeofalltrades codeofalltrades merged commit 0d61f45 into Veil-Project:master Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Algo: Sha256d Bounty: Assigned This issue is being worked on by someone that will earn a bounty reward. Component: Miner Both PoW and PoS block creation Tag: Waiting For QA A pull review is waiting for QA to test the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants