testnet difficulty algorithm integer underflow #1082

1. 
   sirk390 commented at 5:42 PM on April 11, 2012: none

   The new algorithm for testnet difficulty after 15 Feb 2012 behaves incorrectly when pblock->nTime < pindexLast->nTime. As both terms are unsigned, the results of the substraction underflows. The result is greater than nTargetSpacing*2, which causes the difficulty to be set to the minimum. I suppose this could be used to mine a lot of blocks in a row at minimum difficulty.

   See main.cpp line 835:

   // If the new block's timestamp is more than 2* 10 minutes // then allow mining of a min-difficulty block. if (pblock->nTime - pindexLast->nTime > nTargetSpacing*2) return nProofOfWorkLimit;

2. 
   gavinandresen commented at 7:34 PM on April 12, 2012: contributor

   I think there is consensus it is time for a complete testnet reset, with a new genesis block, etc.

3. 
   dooglus commented at 7:55 PM on April 12, 2012: contributor

   @gavinandresen Is that in any way relevant to this bug?

   Resetting testnet wouldn't fix this bug.

   The fix would be changing the line to the following to do away with the unsigned subtraction:

   if (pblock->nTime > pindexLast->nTime + nTargetSpacing*2)
   

4. 
   gavinandresen commented at 8:10 PM on April 12, 2012: contributor

   Sorry for the non-sequitor, it is only relevant in that I think fixing this bug should be combined with a testnet reset.

   Otherwise the testnet chain will be an even bigger mess than it is right now (last I checked there were old nodes mining old chains that didn't include the BIP16/30/difficulty adjustment changes).

5. 
   dooglus commented at 8:12 PM on April 12, 2012: contributor

   Would resetting testnet stop these old nodes mining on old chains? If people aren't updating their clients, they won't even notice the reset.

6. 
   sirk390 commented at 9:01 PM on April 12, 2012: none

   It is relevant as some quite old blocks use this underflow. Fixing it would cause a huge blockchain reverse/fork.

7. 
   Diapolo commented at 9:03 PM on April 12, 2012: none

   @sirk390 Together with a testnet reset this should not be a problem, right?

8. 
   sirk390 commented at 9:15 AM on April 13, 2012: none

   Some statistics might be useful: There are 397 (5%) blocks using this from 46739 to 54673.

   I also did a quick scan about the 15feb2012 difficulty algorithm changes. From the 30 peers I could connect to, 14 are up to date. The other 16 peers are stuck at height 47475 and have a version < 50300. This gives 0.47% of nodes that are updated. There is a fork is at height 46722 until 47475 (753 block long)

   The options I see for the fix: - code fix only: a huge second fork at height 46739, need to redownload the blockchain for all clients. - code fix + special case for blocks [46739-54673]: would create a second fork at height 54673,. - code fix + testnet reset: i'm not sure how many clients will update. 53% probably won't, and some of the 47% in version>50300 probably won't.

   Are there any other reasons for a testnet reset? If not, I'm not sure it's the best option as I don't like to remove all these test case transactions. The fork of 500+ nodes is also a good test scenario.

9. 
   dooglus commented at 9:19 AM on April 13, 2012: contributor

   This gives 0.47% of nodes

   You mean 47%?

10. 
    sirk390 commented at 9:21 AM on April 13, 2012: none

    Yes :)

11. 
    Diapolo commented at 8:54 AM on May 26, 2012: none

    #1392

12. gavinandresen closed this on Jun 29, 2012

    ---

13. suprnurd referenced this in commit 9db6b97979 on Dec 5, 2017
14. dagurval referenced this in commit 77e050d0d4 on May 27, 2018
15. lateminer referenced this in commit 0a2baa686d on Dec 25, 2019
16. Bushstar referenced this in commit 6a26edfde6 on Mar 3, 2020
17. DrahtBot locked this on Sep 8, 2021

Milestone
0.7.0