Bitcoin Forum
May 28, 2017, 03:21:15 AM *
News: If the forum does not load normally for you, please send me a traceroute.
 
   Home   Help Search Donate Login Register  
Pages: [1]
  Print  
Author Topic: [patch] Display additional output, when proof-of-work check fails  (Read 2537 times)
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
December 20, 2010, 03:52:21 AM
 #1

See notes section, below, to understand this format.

Edit:  This is maintained as a patch, at http://yyz.us/bitcoin/patch.bitcoin-pow-fail

Please pull from branch 'pow-fail' of
git://github.com/jgarzik/bitcoin.git pow-fail

to receive the following updates:

Jeff Garzik (1):
      Add -printpowfail to display hash data when proof-of-work verification fails

 main.cpp |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Code:
diff --git a/main.cpp b/main.cpp
index 8db6c39..d0eac7c 100644
--- a/main.cpp
+++ b/main.cpp
@@ -3397,8 +3397,12 @@ bool CheckWork(CBlock* pblock, CReserveKey& reservekey)
     uint256 hash = pblock->GetHash();
     uint256 hashTarget = CBigNum().SetCompact(pblock->nBits).getuint256();
 
-    if (hash > hashTarget)
+    if (hash > hashTarget) {
+       if (GetBoolArg("-printpowfail"))
+            printf("proof-of-work check FAILED...\n  hash: %s\ntarget: %s\n",
+                   hash.GetHex().c_str(), hashTarget.GetHex().c_str());
         return false;
+    }
 
     //// debug print
     printf("BitcoinMiner:\n");


----------------------------------------------------------------------------------------------------------------------------------------

NOTES

Note1:  this is standard Linux kernel git pull request format; often the patches have gone through at least one round of review prior to the pull request, where the kernel subsystem maintainer has reviewed an individual submittor's changes; with this post, I'm collapsing two steps from a larger software project into one, for the sake of brevity and ease of commenting.

Note2:  although the English text says "please pull", implying success, submittors never assume success.  instead, one assumes a basic loop:
  • Step 1: post pull request
  • Step 2: if acceptable, maintainer will pull request.  yay, your change is accepted!
  • Step 3: otherwise, revise based on feedback (or abandon entire approach!), and go to step #1

Note3: yes, the branch name is listed twice in "Please pull" and following line.  The second line is designed to be cut-n-pasted, to make life easier for the person issuing "git pull".

Note4: the section following "receive the following updates" is the output of "git shortlog"

Note5: after that section, the statistics of the diff, as generated by diffstat(1)

Note6: then, what follows is the full patch for public review, quoting and commenting.

Note7: typically you want to pull from third parties onto a merge branch, and then pull merge branch into the main branch, after satisfying yourself that nothing broken or "evil" has been pulled.

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
1495941675
Hero Member
*
Offline Offline

Posts: 1495941675

View Profile Personal Message (Offline)

Ignore
1495941675
Reply with quote  #2

1495941675
Report to moderator
1495941675
Hero Member
*
Offline Offline

Posts: 1495941675

View Profile Personal Message (Offline)

Ignore
1495941675
Reply with quote  #2

1495941675
Report to moderator
1495941675
Hero Member
*
Offline Offline

Posts: 1495941675

View Profile Personal Message (Offline)

Ignore
1495941675
Reply with quote  #2

1495941675
Report to moderator
Advertised sites are not endorsed by the Bitcoin Forum. They may be unsafe, untrustworthy, or illegal in your jurisdiction. Advertise here.
1495941675
Hero Member
*
Offline Offline

Posts: 1495941675

View Profile Personal Message (Offline)

Ignore
1495941675
Reply with quote  #2

1495941675
Report to moderator
1495941675
Hero Member
*
Offline Offline

Posts: 1495941675

View Profile Personal Message (Offline)

Ignore
1495941675
Reply with quote  #2

1495941675
Report to moderator
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
December 20, 2010, 04:10:22 AM
 #2

Rationale:

cpuminer uses a shortcut check for "hash < hashTarget", testing a number of zero bits, then relying on bitcoin iteself to do a proper check.  Several other remote miners behave similarly.  Current bitcoin silently discards work that fails PoW verification.

cpuminer users have found this patch useful in observing the behavior of their remote miners.  When adding -printpowfail, a bitcoin user is guaranteed that each CheckWork() call is logged, either with (a) the familiar proof-of-work-found message, or (b) this new failure message.  I think others will find this minor feature useful as well.

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
Hal
VIP
Sr. Member
*
expert
Offline Offline

Activity: 314



View Profile
December 20, 2010, 06:28:55 AM
 #3

Would it be that hard for external miners to do the extra compare, and only return solutions that actually work? I hate to see too many special-case switches.

Hal Finney
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
December 20, 2010, 07:26:04 AM
 #4

Would it be that hard for external miners to do the extra compare, and only return solutions that actually work? I hate to see too many special-case switches.

That doesn't eliminate the utility of external verification -- being able to see what's going on, on bitcoind's side.  Any number of things could break on the miner's side, making external verification useful.  Maybe the hash check works, but the network code has a problem and garbles data.  Or, the hash check works but we forget to byte-reverse somewhere.

This patch guarantees output regardless of a PoW solution's validity.

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
December 20, 2010, 02:39:24 PM
 #5

I'm with Hal-- do we really need another special-case switch?  Are there significant costs to just always printing when pow fails?

How often do you get the chance to work on a potentially world-changing project?
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
December 20, 2010, 05:22:10 PM
 #6

I'm with Hal-- do we really need another special-case switch?  Are there significant costs to just always printing when pow fails?

Always printing is fine, too.  That is what my original patch did.

Shall I update my pull request to do that?

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
December 21, 2010, 05:38:09 AM
 #7

Now I remember why I added the option:  doesn't the internal miner call CheckWork() if 16 bits are zeroes?  ie. this message would be printed a lot for the internal miner, if added unconditionally, right?

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
Gavin Andresen
Legendary
*
qt
Offline Offline

Activity: 1652


Chief Scientist


View Profile WWW
December 21, 2010, 06:56:13 PM
 #8

Now I remember why I added the option:  doesn't the internal miner call CheckWork() if 16 bits are zeroes?  ie. this message would be printed a lot for the internal miner, if added unconditionally, right?

That's what I meant when I asked "are there significant costs?"

If one of the mining pool folks think this would be useful in a production miner, then I see the value.  If it is only really useful if you're trying to debug a remote miner that isn't working quite right...

How often do you get the chance to work on a potentially world-changing project?
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
December 21, 2010, 08:02:19 PM
 #9

If one of the mining pool folks think this would be useful in a production miner, then I see the value.  If it is only really useful if you're trying to debug a remote miner that isn't working quite right...

Right, and that's happened for all of the 'getwork' miners up to this point.  This is a simple printf() patch that has actually proven useful in the field, not a speculative change with zero users.

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
jgarzik
Legendary
*
qt
Offline Offline

Activity: 1470


View Profile
February 24, 2011, 12:48:04 AM
 #10

Patch updated for latest git.  See top of thread for URL.

Jeff Garzik, bitcoin core dev team and BitPay engineer; opinions are my own, not my employer.
Donations / tip jar: 1BrufViLKnSWtuWGkryPsKsxonV2NQ7Tcj
Pages: [1]
  Print  
 
Jump to:  

Sponsored by , a Bitcoin-accepting VPN.
Powered by MySQL Powered by PHP Powered by SMF 1.1.19 | SMF © 2006-2009, Simple Machines Valid XHTML 1.0! Valid CSS!