Bugfix: Fix testnet-in-a-box use case #5987

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:bugfix_tniab changing 3 files +6 −1
  1. luke-jr commented at 11:56 am on April 9, 2015: member
    Remove testnet checkpoint and allow for older tip blocks
  2. in src/main.cpp: in f976f982d6 outdated 
    1199@@ -1200,7 +1200,7 @@ bool IsInitialBlockDownload()
1200     if (lockIBDState)
1201         return false;
1202     bool state = (chainActive.Height() < pindexBestHeader->nHeight - 24 * 6 ||
1203-            pindexBestHeader->GetBlockTime() < GetTime() - 24 * 60 * 60);
1204+            pindexBestHeader->GetBlockTime() < GetTime() - Params().MaxTipAge());
    laanwj commented at 11:54 am on April 10, 2015:
    Why change this? It only determines whether the client is in initial block download, it shouldn’t prevent the client from accepting blocks.
    luke-jr commented at 11:58 am on April 10, 2015:
    IsInitialBlockDownload disables mining.
    laanwj commented at 11:31 am on April 15, 2015:
    Oh right, that’s ugly…
    jtimon commented at 12:39 pm on April 25, 2015:
    would you mind to declare const CChainParams& chainparams = Params(); at the beginning of the function and use chainparams.MaxTipAge() instead? That will make passing CChainParams as an argument to IsInitialBlockDownload() simpler later.
  3. laanwj added the label Mining on Apr 24, 2015
  4. luke-jr force-pushed on Apr 25, 2015
  5. jtimon commented at 1:11 pm on June 21, 2015: contributor
    ut ACK, though it needs rebase
  6. luke-jr closed this on Jun 23, 2015
  7. luke-jr force-pushed on Jun 23, 2015
  8. luke-jr reopened this on Jun 23, 2015
  9. luke-jr commented at 8:38 am on June 23, 2015: member
    Rebased.
  10. in src/chainparams.cpp: in 2fa12bf8b2 outdated 
    180@@ -179,9 +181,9 @@ class CTestNetParams : public CMainParams {
181 
182         checkpointData = (Checkpoints::CCheckpointData) {
183             boost::assign::map_list_of
184-            ( 546, uint256S("000000002a936ca763904c3c35fce2f3556c559c0214345d31b1bcebf76acb70")),
    jtimon commented at 9:54 am on June 26, 2015:
    Why remove the checkpoint? why not just add the genesis checkpoint as in https://github.com/jtimon/bitcoin/commit/dc1611925dd36f07d1cf3412237610b864a99bcc ? Also, why not just use consensus.hashGenesisBlock in the genesis checkpoint instead of repeating the hardcoded value?
    jtimon commented at 10:02 am on June 26, 2015:
    In fact, why not add the genesis checkpoint (the genesis block, the only checkpoint that cannot be reorged) to the mainchain too (and while at it use consensus.hashGenesisBlock for regtest’s gensis checkpoint for uniformity).
  11. jtimon commented at 12:39 pm on June 29, 2015: contributor
    Any thoughts on the nits and proposed changes to this PR?
  12. jgarzik commented at 6:21 pm on September 15, 2015: contributor
    Along the lines of @jtimon ’s comments - a genesis checkpoint does not appear to be needed.
  13. jtimon commented at 7:01 pm on September 15, 2015: contributor
    @jgarzik I’m actually saying the opposite: the genesis checkpoint that can be trusted (genesis block) is good, but why remove any existing checkpoint in testnet3 ?
  14. jgarzik commented at 8:06 pm on September 15, 2015: contributor

    @jtimon That was covered in the opening PR description? “allow for older tip blocks”

    If we don’t want that then sure, keep the checkpoint.

  15. jtimon commented at 8:35 pm on September 15, 2015: contributor

    The op says:

    Remove testnet checkpoint

    why?

    …and allow for older tip blocks

    I want this (like in https://github.com/jtimon/bitcoin/commit/dc1611925dd36f07d1cf3412237610b864a99bcc). jgarzik seems not to want this for some reason.

  16. jgarzik commented at 8:46 pm on September 15, 2015: contributor
    I have no real opinion. Just moving things along.
  17. luke-jr commented at 0:32 am on September 20, 2015: member

    Checkpoints beyond the genesis block are a problem because testnet-in-a-box networks won’t have those blocks.

    MaxTipAge is needed because otherwise mining the first (non-genesis) block is impossible on such a network.

  18. luke-jr force-pushed on Sep 20, 2015
  19. jtimon commented at 8:31 am on September 21, 2015: contributor

    Checkpoints beyond the genesis block are a problem because testnet-in-a-box networks won’t have those blocks.

    I see. They could also disable checkpoints in their test, but I don’t have a strong opinion: I’m fine removing it if that’s the motivation. What about the rest of my feedback (ie making sure all ghains have a genesis checkpoint and using hardcoded values as little as possible)?

  20. luke-jr commented at 3:48 pm on September 21, 2015: member
    @jtimon I don’t care about that proposed change, but it’s certainly not a bugfix or in the scope of this PR…
  21. laanwj commented at 4:17 pm on September 22, 2015: member

    Instead of removing the checkpoint, users of testnet-in-a-box could specify the option -checkpoints=0 - right?

    I don’t see a simple alternative solution for the tip age, though I still think having it per network is hacky.

  22. jtimon commented at 5:29 pm on September 22, 2015: contributor
    +1 on @laanwj ’s suggestion to avoid removing testnet3’s checkpoint. @luke-jr I’m just asking to add the genesis checkpoint in main, just like you are adding it to testnet (and already exists on regtest), but if you want to leave that for later (see #6382), that’s fine too.
  23. luke-jr commented at 8:22 pm on September 22, 2015: member
    @laanwj If I remove the checkpoint changes here, can we merge the other part with an understanding it will go away when a better solution is found?
  24. laanwj commented at 4:52 pm on September 23, 2015: member
    @luke-jr fine with me
  25. laanwj commented at 12:26 pm on September 29, 2015: member
    @luke-jr ping
  26. Bugfix: Allow mining on top of old tip blocks for testnet (fixes testnet-in-a-box use case) e761d7a94f
  27. luke-jr force-pushed on Sep 29, 2015
  28. luke-jr commented at 7:47 pm on September 29, 2015: member
    Sorry for the delay, done.
  29. jgarzik commented at 10:22 am on October 1, 2015: contributor
    ut ACK
  30. laanwj commented at 12:06 pm on October 1, 2015: member
    utACK
  31. laanwj merged this on Oct 1, 2015
  32. laanwj closed this on Oct 1, 2015
  33. laanwj referenced this in commit 4899a04c24 on Oct 1, 2015
  34. laanwj referenced this in commit 64360f1304 on Dec 14, 2015
  35. deadalnix referenced this in commit d9ec1a7db1 on Jan 11, 2017
  36. deadalnix referenced this in commit 27d238fdb1 on Jan 11, 2017
  37. deadalnix referenced this in commit a193ba35df on Jan 15, 2017
  38. deadalnix referenced this in commit 890e1c1dda on Jan 16, 2017
  39. deadalnix referenced this in commit 398d02958b on Jan 17, 2017
  40. deadalnix referenced this in commit 3015c86761 on Jan 19, 2017
  41. deadalnix referenced this in commit e442224c4b on Jan 19, 2017
  42. deadalnix referenced this in commit 36cf0f2266 on Jan 19, 2017
  43. deadalnix referenced this in commit 1164b55292 on Jan 26, 2017
  44. deadalnix referenced this in commit 85a832d6a6 on Feb 12, 2017
  45. sickpig referenced this in commit 52db63687f on Feb 25, 2017
  46. deadalnix referenced this in commit c2a143452c on Feb 27, 2017
  47. deadalnix referenced this in commit fa68b4bcd3 on Feb 27, 2017
  48. MarcoFalke locked this on Sep 8, 2021


luke-jr laanwj jtimon jgarzik

Labels
Mining

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me