block-sync: poll: make best known chain tip optional

[valentinewallace]

For clients who are bootstrapping and have no best known tip. Unless I'm missing something here.

[TheBlueMatt]

Hmm, how are you trying to use Poll, specifically? The synchronize_listeners method in block-sync/init takes a BlockSource instead, which doesn't require a previous tip.

[valentinewallace]

Hmm, how are you trying to use Poll, specifically? The synchronize_listeners method in block-sync/init takes a BlockSource instead, which doesn't require a previous tip.

SpvClient::new requires a chain tip. And if we're starting a fresh node and had no reason to call synchronize_listeners, then we don't have one afaict.

[codecov]

Codecov Report

Merging #807 (a1b3de1) into main (e77b160) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #807      +/-   ##
==========================================
+ Coverage   91.00%   91.05%   +0.04%     
==========================================
  Files          48       48              
  Lines       25480    25492      +12     
==========================================
+ Hits        23188    23211      +23     
+ Misses       2292     2281      -11     

Impacted Files	Coverage Δ
lightning-block-sync/src/lib.rs	95.37% <100.00%> (ø)
lightning-block-sync/src/poll.rs	92.55% <100.00%> (+0.50%)	⬆️
lightning/src/ln/functional_tests.rs	97.13% <0.00%> (+0.18%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e77b160...a1b3de1. Read the comment docs.

[TheBlueMatt]

And if we're starting a fresh node and had no reason to call synchronize_listeners

Ah, good point. I wonder if we're better off just passing a None in to SpvClient instead, indicating a fresh node. What do you think @jkczyz ?

[jkczyz]

And if we're starting a fresh node and had no reason to call synchronize_listeners

Ah, good point. I wonder if we're better off just passing a None in to SpvClient instead, indicating a fresh node. What do you think @jkczyz ?

How about another utility in init that given a Network returns a ValidatedBlockHeader of the genesis block?

[TheBlueMatt]

How about another utility in init that given a Network returns a ValidatedBlockHeader of the genesis block?

That would end up downloading the full chain, no? Ideally we'd just start from the current tip because we don't care about any of the historical data.

[jkczyz]

That would end up downloading the full chain, no? Ideally we'd just start from the current tip because we don't care about any of the historical data.

True. Giving None to SpvClient would be preferable then I suppose.

[TheBlueMatt]

We could also do a util function in init that returns the current best block as a ValidatedBlockHeader, which would maybe be more explicit in the sense that you have to call something that has the word "fresh_node" in it or so.

[jkczyz]

We could also do a util function in init that returns the current best block as a ValidatedBlockHeader, which would maybe be more explicit in the sense that you have to call something that has the word "fresh_node" in it or so.

Hmm... ok had some time to think about this. Doesn't calling synchronize_listeners without any listeners already do what we want?

We could make a utility that does this though technically it would ignore the Network parameter. Alternatively, we could refactor the first four lines of synchronize_listeners into a separate pub function -- essentially what you suggested -- and use it in synchronize_listeners. Maybe call it validate_best_block_header given calling it fresh_node wouldn't be applicable in this context.

[TheBlueMatt]

Either sounds good to me, seems like it would have a similar public API.

[jkczyz]

Superseded by #818.