Always print full hashes (tx, block, inv) #2478

pull sipa wants to merge 1 commits into bitcoin:master from sipa:fullhash changing 6 files +44 −54

sipa commented at 1:45 AM on April 7, 2013: member

It has bothered me for a while that you can't just use hashes from debug.log in query commands that require a full hash.
Always print full hashes (tx, block, inv) 1c06aa98c6
gmaxwell commented at 1:55 AM on April 7, 2013: contributor

ACK ACK ACK ACK ACK (I carry a patch on and off that does this, but it breaks from time to time and I get lazy and don't forward port it... and then regret it when something weird happens)
BitcoinPullTester commented at 2:22 AM on April 7, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/1c06aa98c63fff02679d446588fad06ae8cd706f for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
jgarzik commented at 3:42 AM on April 7, 2013: contributor

ACK -- although I think it is a symptom of lacking a partial hash search ability, such as that is built into git.
gmaxwell commented at 3:50 AM on April 7, 2013: contributor

Partial has searching requires a more expensive index than exact matching and isn't needed for any of the network protocol or basic functionality of the software. Though what we use today can support it, getting stuck with it would be unfortunate when it only saves us some space in logs.
rebroad commented at 2:45 PM on April 8, 2013: contributor

Can this be implemented as a command line option to either display the full hash or an abbreviated hash please? I far prefer block hashes to be 15 characters long i.e. substr(5,15), and txs to be 10 chars long.
sipa commented at 2:46 PM on April 8, 2013: member

I personally don't care about the esthetics of the debug.log layout. What do others think?
in src/main.h:None in 1c06aa98c6
```
 194 | @@ -195,11 +195,6 @@
 195 |  
 196 |  
 197 |  
 198 | -static inline std::string BlockHashStr(const uint256& hash)
```
rebroad commented at 2:47 PM on April 8, 2013:

I thought this was only recently introduced in a pull request recently. Is some kind of code fight going on here?

jgarzik commented at 2:52 PM on April 8, 2013:

It was recently introduced, simply as a way of transitioning without breaking. Removing it is not a problem, assuming people ACK this patchset.

sipa commented at 2:52 PM on April 8, 2013:

Luke had a pullreq some time ago which introduced this abstraction, yes. I think it's overkill. Also, I want to extend it to transaction hashes too.

rebroad commented at 11:53 AM on April 10, 2013:

If we want to give people a choice about how long the hashes are (using a setting in bitcoin.conf), then surely this function would need to be re-introduced again, wouldn't it?

laanwj commented at 1:07 PM on April 10, 2013:

ACK. Just show the full thing. And please let's not bikeshed about this. It's a debug log and those first and foremost should be complete.
gavinandresen commented at 3:27 PM on April 8, 2013: contributor

ACK
gavinandresen referenced this in commit f158e363a4 on Apr 9, 2013
gavinandresen merged this on Apr 9, 2013
gavinandresen closed this on Apr 9, 2013
sipa deleted the branch on Apr 9, 2013
laudney referenced this in commit e4779d9f0d on Mar 19, 2014
DrahtBot locked this on Sep 8, 2021

Contributors