[patch] Display additional output, when proof-of-work check fails

[jgarzik]

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.

[1713248475]

1713248475

[jgarzik]

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.

[Hal]

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.

[jgarzik]

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.

[Gavin Andresen]

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

[jgarzik]

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?

[jgarzik]

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?

[Gavin Andresen]

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...

[jgarzik]

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.

[jgarzik]

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