Fix for GetBlockValue() after block 13,440,000 #3842

Forces the block reward to zero when right shift in GetBlockValue() is undefined, after 64 reward halvings (block height 13,440,000).

NACK

We should make use of the ~250 years we have to study this issue more carefully.

I'm sure the altcoin creators can figure this out of themselves right?

UUID guy is giving altcoin devs way too much credit.

Code change looks fine to me. Any value of halvings>32 will already bring the subsidy to 0, so regarding it as zero for halvings>=64 is safe.

Are we going to merge this, or not? It appears safe to me but as it involves critical part of the consensus code I don't want to make this decision on my own. @sipa @gmaxwell @gavinandresen

@ditto-b Could you please do up a test script that actually exercises this code? regtest mode has a shortened halving interval and a trivial PoW so creating the blocks and submitting them locally shouldn't be too hard. You can use https://github.com/petertodd/python-bitcoinlib/ if you know python - it has both the CBlock/CTransaction stuff to make the blocks, and easy access to the RPC interface. I'd be happy to help review it for you.

We do have a test for the function (subsidy_limit_test) but it only runs until block 7 000 000. Would be trivial to increase, though.

@laanwj Gah! I was trying to get @ditto-b to write a test harness for me to use to test the upcoming Litecoin soft-fork.

ACK. This is an implementation of BIP42.

Though we should make the trivial change of improving the test too.

I think this is a great fix. When I first read about Bitcoin I loved the idea that it has finite coins, and this is how it should be in my opinion.

:S in 250 years we'll be using qubits, flying around the galaxy and mating with space honeys

i'm glad you have eternal faith in c++

Code is moving away from it's comments. Keep the comment about halving with the actual halving math (l 1085 -> 1079)

This is clearly a bug and one serious enough that I believe if we do not fix it in a timely manner it could destroy confidence in the Bitcoin network.

Timely manner? ... Well I'm absolutely confident that it will be fixed sometime in the hundreds of years required before it matters.

The longer we delay, the more likely it is that a miner alive today could live until this bug occurs. This gives them a greater incentive to not implement the fix. This is particularly true for any toddlers or infants in charge of sizeable farms.

It doesn't matter what miners implement, it matters what the rest of bitcoin nodes run. If you mine with rules inconsistent with the network you are simply not mining.

@aceat64 So, have you tested this pull? Have you updated the unit test which is the only thing that's holding this back from being merged? Please do something useful instead of distracting people by screaming hurry hurry hurry.

I feel as though the tone of my message has been misunderstood. I attempted to clarify with my mention of toddlers and infants running large mining farms.

Thanks. :)

I'm not particularly concerned about what happens to bitcoin in 250 years as I don't expect to be around by then. Is this really likely to be the PR disaster that some people are suggesting?!

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/5cfd3a70a67ba707a8f074a1730724a6e86353b8 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.