Improve IBD check

[justinmoon]

With the following regtest getblockchaininfo result bwt gets stuck at this loop. I don't know why Bitcoin Core is reporting initialblockdownload as true ... perhaps we should just compare blocks and headers ourselves? Although that could be prone to error the moment the node starts up and hasn't fetched new headers yet.

$ bitcoin-cli -regtest getblockchaininfo
{
  "chain": "regtest",
  "blocks": 106,
  "headers": 106,
  "bestblockhash": "584a14a4d71fa98be9297f7ff70754533f6778b9e3ab4ba9fb2340fbe3b1e88b",
  "difficulty": 4.656542373906925e-10,
  "mediantime": 1592948117,
  "verificationprogress": 1,
  "initialblockdownload": true,
  "chainwork": "00000000000000000000000000000000000000000000000000000000000000d6",
  "size_on_disk": 33373,
  "pruned": false,
  "softforks": {
    "bip34": {
      "type": "buried",
      "active": false,
      "height": 500
    },
    "bip66": {
      "type": "buried",
      "active": false,
      "height": 1251
    },
    "bip65": {
      "type": "buried",
      "active": false,
      "height": 1351
    },
    "csv": {
      "type": "buried",
      "active": false,
      "height": 432
    },
    "segwit": {
      "type": "buried",
      "active": true,
      "height": 0
    },
    "testdummy": {
      "type": "bip9",
      "bip9": {
        "status": "defined",
        "start_time": 0,
        "timeout": 9223372036854775807,
        "since": 0
      },
      "active": false
    }
  },
  "warnings": ""
}

[shesek]

This is happening because bitcoind considers itself in IBD when no new blocks were received for more than nMaxTipAge. Generating a new block should get it out of IBD mode. (previously also brought up in #35 (comment))

I'm not sure what's the right way to handle this. We could check for headers == blocks instead (I actually have this commented out in the code), but this might have unintended consequences, as you mentioned. IsInitialBlockDownload does a bunch more checks that may be necessary.

electrs is also using initialblockdownload (and is behaving similarly annoying with stale regtest chains), perhaps @romanz has some insight on how that compares to using headers == blocks?

One option that seems safe is keeping the current check for non-regtest chains, and using headers == blocks for regtest only.

[shesek]

One option that seems safe is keeping the current check for non-regtest chains, and using headers == blocks for regtest only.

I went ahead and implemented that in 49dc380. @justinmoon Can you confirm this works for you as intended?

[romanz]

electrs is also using initialblockdownload (and is behaving similarly annoying with stale regtest chains), perhaps @romanz has some insight on how that compares to using headers == blocks?

Yes, handling regtest "staleness" indeed an issue...
I think that the solution in 49dc380 should work well.

[shesek]

Thanks @romanz!

[justinmoon]

I haven't tested this, but I wrote the exact same code locally a few days ago and it fixed it. This looks good.