Use full block hash as unique identifier in debug.log #1670

pull luke-jr wants to merge 2 commits into bitcoin:master from luke-jr:blksubstr changing 5 files +38 −27

luke-jr commented at 7:06 AM on August 13, 2012: member

This is a rebase and cleanup of #1426

Note that while these bits are less significant numerically, they are more useful because they are likely to be unique (whereas the most significant bits will generally be zero)
BitcoinPullTester commented at 3:21 PM on August 13, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0e6b9c809b4941d3062f092743df8d8e5588e7c9 for binaries and test log.
sipa commented at 1:03 PM on August 17, 2012: member

I believe @gmaxwell argued in an earlier request for just using the full string. I wouldn't mind, and it enables using getblock directly, for example, without extra lookup. It's still a debug log file, right?
in src/main.h:None in 0e6b9c809b outdated
```
 113 | @@ -114,6 +114,8 @@
 114 |  
 115 |  
 116 |  
 117 | +#define BlockSubstr(hash)  hash.ToString().substr(48)
```
laanwj commented at 6:58 PM on August 20, 2012:

Please use an inline function, not a macro
laanwj commented at 7:03 PM on August 20, 2012: member

How long is the full string? 64? I suppose so. This patch makes it print 48 characters, so I guess you could just as well go all the way print the full string.
luke-jr commented at 7:13 PM on August 20, 2012: member

No, this patch skips 48 characters, so prints 16. I don't care either way, and it seems there's support for the full hash, so might as well do that anyway...
in src/main.h:None in 508c455bc3 outdated
```
 113 | @@ -114,6 +114,13 @@
 114 |  
 115 |  
 116 |  
 117 | +static inline
```
Diapolo commented at 8:10 PM on August 20, 2012:

Why are you always trying to introduce new and weird coding conventions? static inline std::string BlockSubstr(const uint256& hash) would do the job, no?

sipa commented at 11:40 AM on August 22, 2012:

That's not a convention we use, but in some places it is. Still, can you change it, @luke-jr?

EDIT: apparently it's used in some test cases already. Nevermind.

laanwj commented at 11:45 AM on August 22, 2012:

I'm also for changing it. Splitting up lines can be useful when they get too long for convenient reading, but this really breaks the flow of reading.

Diapolo commented at 3:51 PM on September 1, 2012:

I guess this was changed by another dev and we need a rebase for this pull.
BitcoinPullTester commented at 9:39 AM on August 21, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/508c455bc337c3a56d1499c5cd691114427c8f46 for binaries and test log.
rebroad commented at 3:11 PM on September 1, 2012: contributor

So.. this patch now makes the lines in debug.log longer instead of shorter?
jgarzik commented at 4:07 PM on September 4, 2012: contributor

ACK save for bikeshedding: "BlockSubstr" is a misleading name, because the end result of this changeset is not a substring, but rather the full hash string. Maybe "BlockHashStr".
luke-jr commented at 8:39 PM on September 4, 2012: member

Renamed.
rebroad commented at 12:08 PM on September 5, 2012: contributor

How long will the line in the debug.log file now be with this change? What width of window is needed for it to look sufficiently neat? Is it worth adding a command line option to specify the length or format of the block string perhaps?
luke-jr commented at 1:21 PM on September 5, 2012: member

It's a debug log; it doesn't have to look neat. NACK on a command line option...
laanwj commented at 2:32 PM on September 5, 2012: member

Agree with @luke-jr here. The point of the log is simply to give as much as possible info for debugging issues.
BitcoinPullTester commented at 5:23 PM on September 7, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c97992845777f0625552153b5644f556fa15c8f4 for binaries and test log.
sipa commented at 11:56 AM on September 21, 2012: member

ACK
sipa commented at 11:42 PM on October 28, 2012: member

Needs rebase.
Abstract block hash substr extraction (for debug.log) into BlockHashStr inline f3a84c3a6b
Change block references in debug.log to full hash instead of just 0..20 c34a32699e
luke-jr commented at 9:21 PM on November 13, 2012: member

Rebased
sipa commented at 9:48 PM on November 13, 2012: member

Hmm just realized that this is only for blocks. Any reason not to print full hashes for transactions?
jgarzik referenced this in commit 3ef292dc7d on Nov 16, 2012
jgarzik merged this on Nov 16, 2012
jgarzik closed this on Nov 16, 2012
laudney referenced this in commit d3fe9c7b58 on Mar 19, 2014
luke-jr deleted the branch on Jan 1, 2015
suprnurd referenced this in commit ece884994e on Dec 5, 2017
suprnurd referenced this in commit a5f99ef2f5 on Dec 5, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#1426 Show more significant bytes of block hashes.