Use full block hash as unique identifier in debug.log #1670
Conversation
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0e6b9c809b4941d3062f092743df8d8e5588e7c9 for binaries and test log. |
|
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? |
| @@ -114,6 +114,8 @@ | |||
|
|
|||
|
|
|||
|
|
|||
| #define BlockSubstr(hash) hash.ToString().substr(48) | |||
There was a problem hiding this comment.
Please use an inline function, not a macro
|
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. |
|
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... |
| @@ -114,6 +114,13 @@ | |||
|
|
|||
|
|
|||
|
|
|||
| static inline | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I guess this was changed by another dev and we need a rebase for this pull.
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/508c455bc337c3a56d1499c5cd691114427c8f46 for binaries and test log. |
|
So.. this patch now makes the lines in debug.log longer instead of shorter? |
|
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". |
|
Renamed. |
|
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? |
|
It's a debug log; it doesn't have to look neat. NACK on a command line option... |
|
Agree with @luke-jr here. The point of the log is simply to give as much as |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c97992845777f0625552153b5644f556fa15c8f4 for binaries and test log. |
|
ACK |
|
Needs rebase. |
|
Rebased |
|
Hmm just realized that this is only for blocks. Any reason not to print full hashes for transactions? |
Use full block hash as unique identifier in debug.log
Use full block hash as unique identifier in debug.log
- Convert satoshis to duffs - Correct order of fields in GetAddressUtxos - Add missing field from GetAddressDeltas (blockindex) - Add HelpExamples to gettxoutproof and verifytxoutproof
…bitcoin#1711) "duffs" -> "satoshis" for field names
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)