Skip to content

Conversation

@rebroad
Copy link
Contributor

@rebroad rebroad commented Jun 5, 2012

On Mon Aug 16 20:55:54 2010, Satoshi modified the client to display 20 bytes of the block hash (up from 16 bytes). This pull request increases the number of significant bytes by 5, and reduces the number of insignificant bytes (almost always zeros) by 10, resulting in saved space in debug.log and improved uniqueness of block hashes.

@TheBlueMatt
Copy link
Contributor

I'd like to keep one leading 0 on diff-1 blocks, instead of removing all the 0s, which would leave only 6 0s on current-diff blocks, which IMHO is acceptable, but ACK on the idea.

@gavinandresen
Copy link
Contributor

I like the at-least-one leading 0 on min-difficulty blocks idea; I've spent a lot of time staring at min difficulty testnet block hashes.

@gavinandresen
Copy link
Contributor

One downside of this change: it might break blockexplorer.com's block lookup (please test, I know it works with the existing lots-of-leading-zeroes format).

@rebroad
Copy link
Contributor Author

rebroad commented Jun 6, 2012

I see what you mean re testnet. Perhaps I should code it so that it's one leading zero whatever the situation...?

@sipa
Copy link
Member

sipa commented Jun 8, 2012

Solution: use the low bits of block hashes to identify them, instead of high bits which contain several zeroes?

@luke-jr
Copy link
Member

luke-jr commented Jun 8, 2012

I definitely prefer showing the low bits over high ones.

@TheBlueMatt
Copy link
Contributor

Agreed, as long as someone pings theymos et al to get bbe/blockchain.info updated to support low bit-searching implemented before release.

@luke-jr
Copy link
Member

luke-jr commented Jun 9, 2012

BBE doesn't care which bits you use. Blockchain.info doesn't accept partial matches/searches at all.

@TheBlueMatt
Copy link
Contributor

Ah, well then I guess we should just switch to low bits.

@sipa
Copy link
Member

sipa commented Jun 14, 2012

Using the 14 lowest hex characters of a block as identifier should suffice for 100 years (0.2% chance for a collision after 5.46 million blocks). So I'd say use the 16 lowest ones.

@gmaxwell
Copy link
Contributor

I'd somewhat prefer to show the full values, because thats what the getblock rpc needs. If we're concerned about space we should create rotation that doesn't stink.

@rebroad
Copy link
Contributor Author

rebroad commented Jun 14, 2012

@sipa it's just a debug.log. I doubt anyone will have a debug.log file as long as 100 years, so 14 characters would be an extreme overkill.

@gmaxwell it doesn't currently show full values and never has. The current requirement seems to be that the numbers don't want to be duplicated within the entirity of the debug.log file. For that, the last 14 hex characters are more than sufficient (as sipa pointed out).

@luke-jr
Copy link
Member

luke-jr commented Jun 18, 2012

@rebroad Are you going to update this to show the least-significant (end/tail) N?

@rebroad
Copy link
Contributor Author

rebroad commented Jun 21, 2012

Yes, very likely, when i next get access to a computer (next week sometime).

@rebroad
Copy link
Contributor Author

rebroad commented Jul 2, 2012

@luke-jr can I ask your opinion - is it better to hardcode the substr numbers or instead do it in a way that uses the length of the string?

@luke-jr
Copy link
Member

luke-jr commented Jul 2, 2012

I'd just go with substr(504)

@rebroad
Copy link
Contributor Author

rebroad commented Jul 2, 2012

@luke-jr modified (not yet tested - compiling)

@sipa
Copy link
Member

sipa commented Jul 3, 2012

wth? 504? hex hashes are only 64 characters long.

@sipa
Copy link
Member

sipa commented Jul 3, 2012

@rebroad I prefer the reported hashes in the debug file to be globally and near-forever usable as unique identifiers. I'd just use the low 64 bits of the hash (i.e., the last 16 hex characters). .substr(48) i suppose.

@luke-jr
Copy link
Member

luke-jr commented Jul 3, 2012

... yeah, not sure how I got 504 (except I know I forgot to divide by 8)... long day :(

I meant .substr(56), but I don't mind 48

@gmaxwell
Copy link
Contributor

gmaxwell commented Jul 3, 2012

@rebroad I know it doesn't currently show full values.

@sipa I still think it should show the full value. The partial values are almost useless because there is no way to getblock by them. I don't think carrying another index for partial block values names sense, nor do I think precluding hash tables that require exact lookups makes sense. Reducing them by 1/4th while breaking lookup with getblock isn't a worthwhile tradeoff.

(Of, course, I'll continue to patch my nodes to log the full value regardless, but I think its a reasonable default).

@rebroad
Copy link
Contributor Author

rebroad commented Jul 3, 2012

updated. now substr(48) (effectively). Next.. shall we shorten the work= field....?!

@sipa
Copy link
Member

sipa commented Jul 3, 2012

How about using the unit GHy? (gigahash year). We're currently at around 11874.5 GHy.

@rebroad
Copy link
Contributor Author

rebroad commented Jul 3, 2012

@sipa how do you turn the current value into GHy?

@sipa
Copy link
Member

sipa commented Jul 4, 2012

Divide the bnWork number by 31556926000000000 (1 billion times the number of seconds in a year).

@rebroad
Copy link
Contributor Author

rebroad commented Jul 4, 2012

@sipa Shall I add that change to this pull, or a separate pull (which would need to go in after this one?)

@midnightmagic
Copy link
Contributor

Currently there is no way for a user to go from the entries in the debug.log, using bitcoind calls only, to useful information. Is there no intention of correcting the api calls so they accept whatever it is we're dumping into debug.log? This way we can do investigation without writing our own extensive tools or being forced to visit a website? How is it that you guys debug problematic blocks? Do you maintain extensive patchsets of your own that don't depend on behaviour in the base client?

@jgarzik
Copy link
Contributor

jgarzik commented Aug 1, 2012

Weak ACK, mostly don't care.

BLOCKSUBSTR should be in a separate, zero-behavior-change commit.

@Diapolo
Copy link

Diapolo commented Aug 1, 2012

BLOCKSUBSTR could take a little comment IMHO.

@luke-jr
Copy link
Member

luke-jr commented Aug 1, 2012

While we're fence-painting, it seems to me hash.ToString().substr(BLOCKSUBSTR).c_str() could be reduced to hash.BlockSubstr().c_str()

@BitcoinPullTester
Copy link

@laanwj
Copy link
Member

laanwj commented Aug 20, 2012

Closing this in favor of #1670

@laanwj laanwj closed this Aug 20, 2012
suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017
* Modify MasternodeRateCheck to support updating buffers only on failure

* Update rate check buffer only when fAddToSeen is true
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 22, 2019
* pad IBLT to compensate for missing transactions

* refine excess items when receiver has more txs than sender

* simplify graphene arithmetic
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.