Start9Labs / bitcoind-startos

wrapper for building bitcoind.s9pk
Other
14 stars 20 forks source link

add full archival health check to bitcoind #23

Closed chrisguida closed 2 years ago

chrisguida commented 2 years ago

This is to allow c-lightning to check to see if bitcoind is a full archival node so it can know whether it needs to use proxy instead

chrisguida commented 2 years ago

@ProofOfKeags @dr-bonez is this still needed?

dr-bonez commented 2 years ago

It might be. It depends on whether switching from pruned to full archival is a different kind of resync than the base one, and whether we can measure it.

chrisguida commented 2 years ago

Making a health check sounds much easier than that

chrisguida commented 2 years ago

That being said, pruned: false comes back in getblockchaininfo, bwt uses this to determine if bitcoind is pruned. Is that sufficient for c-lightning?

ProofOfKeags commented 2 years ago

and whether we can measure it.

I don't think we can easily, the block data gets downloaded in the background if it's already synced and there is just a pruned message that comes out over the RPC if the block is missing. I don't think it exposes the internal tracking of what it has and what it doesn't.

chrisguida commented 2 years ago

and whether we can measure it.

I don't think we can easily, the block data gets downloaded in the background if it's already synced and there is just a pruned message that comes out over the RPC if the block is missing. I don't think it exposes the internal tracking of what it has and what it doesn't.

Not sure I understand this, why do we need more info than just pruned: true/false?

ProofOfKeags commented 2 years ago

This goes into whether or not health checks are dynamic or static. The current understanding of the purpose of healthchecks is to use them for things that can feasibly change over the lifetime of the program. As such I think that this ticket should be closed as Won't Fix because it does not satisfy that property.

My comment about whether or not we can measure it is a different one. If we were to add a full-archival health check to bitcoind it should be to check on the status of all of the blockdata being fundamentally available. This satisfies the above requirement because it can feasibly change over the lifetime of the program, however, actually accomplishing that measurement may be impossible right now.

chrisguida commented 2 years ago

Ok, sounds good. Closing per @ProofOfKeags's comment and @dr-bonez's suggestion to have it be a config pointer instead of a health check: https://github.com/Start9Labs/c-lightning-wrapper/issues/17#issuecomment-987126751

chrisguida commented 2 years ago

@ProofOfKeags Just out of curiosity, how would bitcoind get into a state where it says it's archival in getblockchaininfo but it's not actually archival?

dr-bonez commented 2 years ago

Proxy does that if fetch blocks is enabled

ProofOfKeags commented 2 years ago

Just out of curiosity, how would bitcoind get into a state where it says it's archival in getblockchaininfo but it's not actually archival?

If it hasn't finished redownloading all the blocks yet.

chrisguida commented 2 years ago

@ProofOfKeags but then it says initialblockdownload: true:

# docker exec -it bitcoind.embassy bitcoin-cli getblockchaininfo
{
  "chain": "main",
  "blocks": 8814,
  "headers": 713112,
  "bestblockhash": "00000000bbdf2c879ae84da6d38f018bf29f4e08732f5a817069165ad484d024",
  "difficulty": 1,
  "mediantime": 1238106210,
  "verificationprogress": 1.263223867161213e-05,
  "initialblockdownload": true,
  "chainwork": "0000000000000000000000000000000000000000000000000000226f226f226f",
  "size_on_disk": 2496266,
  "pruned": false,
  "softforks": {
    "bip34": {
      "type": "buried",
      "active": false,
      "height": 227931
    },
    "bip66": {
      "type": "buried",
      "active": false,
      "height": 363725
    },
    "bip65": {
      "type": "buried",
      "active": false,
      "height": 388381
    },
    "csv": {
      "type": "buried",
      "active": false,
      "height": 419328
    },
    "segwit": {
      "type": "buried",
      "active": false,
      "height": 481824
    },
    "taproot": {
      "type": "bip9",
      "bip9": {
        "status": "defined",
        "start_time": 1619222400,
        "timeout": 1628640000,
        "since": 0,
        "min_activation_height": 709632
      },
      "active": false
    }
  },
  "warnings": ""
}
ProofOfKeags commented 2 years ago

even if you go from pruned to unpruned after the sync has finished?

chrisguida commented 2 years ago

Just tested this on my laptop, yes.

However, I just noticed that bitcoind will refuse to start when you switch from pruned to unpruned, and requires you to restart with the -reindex flag. Will test to see if the manager script is properly handling this case.