btcsuite
blockchain, netsync: implement a complete headers-first download during ibd
Change Description
Right now the headers-first download is only based off of the checkpoints and is thus limited to the last checkpoint. The newly implemented headers-first download will always download headers-first and will validate them to see if the headers connect and have proper proof of work.
Then the block download will be based off of the verified headers. This now eliminates any potential downloading of txs or orphan blocks during ibd. It also makes future parallel block download much better as a parallel block download can only happen for blocks we already have headers for.
~~It's not yet put into the code yet but this allows the node to also receive block headers instead of invs during block propagation.~~ I'll do this in a follow up later.
Steps to Test
- Perform ibd on any of the networks and see that the node syncs up fine.
- Check that the newly added function tests are correct.
Pull Request Checklist
Testing
- [x] Your PR passes all CI checks.
- [x] Tests covering the positive and negative (error paths) are included.
- [ ] Bug fixes contain tests triggering the bug to prevent regressions.
Code Style and Documentation
- [x] The change is not insubstantial. Typo fixes are not accepted to fight bot spam.
- [x] The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
- [ ] Commits follow the Ideal Git Commit Structure.
- [ ] Any new logging statements use an appropriate subsystem and logging level.
📝 Please see our Contribution Guidelines for further guidance.
Pull Request Test Coverage Report for Build 19373443826
Details
- 139 of 254 (54.72%) changed or added relevant lines in 6 files are covered.
- 65 unchanged lines in 6 files lost coverage.
- Overall coverage increased (+0.5%) to 55.433%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| blockchain/chainio.go | 1 | 3 | 33.33% |
| blockchain/chain.go | 11 | 22 | 50.0% |
| netsync/manager.go | 40 | 142 | 28.17% |
| <!-- | Total: | 139 | 254 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| mempool/mempool.go | 1 | 66.56% |
| btcutil/gcs/gcs.go | 3 | 81.25% |
| database/ffldb/blockio.go | 4 | 88.81% |
| rpcclient/infrastructure.go | 4 | 47.8% |
| netsync/manager.go | 18 | 7.05% |
| blockchain/chainio.go | 35 | 69.75% |
| <!-- | Total: | 65 |
| Totals | |
|---|---|
| Change from base Build 19121431262: | 0.5% |
| Covered Lines: | 31425 |
| Relevant Lines: | 56690 |
💛 - Coveralls
Tested in the wild, and does what it says on the tin:
One thing I noticed though is that the peers may D/C us along the way, so we rotate through a few peers to fetch all the headers. This is testnet3 , there's quite a lot of headers (over 4 million). Are we hitting something like default per-peer upload limit for bitcoind?
IIUC this is a stepping stone for multi-peer header download, so perhaps that'll be sorted out as we'll fetch blocks of headers from many peers in parallel.
