Require chain::Listen impls in block sync be Send + Sync

[TheBlueMatt]

Users who want to use lightning-block-sync's init module would
be reasonable in wanting to use it in a multithreaded environment,
however because it takes a list of listeners as dyn chain::Listen
without any Send or Sync bound they fail in doing so.

Here we add a Send + Sync bound, requiring any listeners be both.
This could be less generic for users with their own chain::Listen
listener that is not Send or Sync, but given multi-threading
support is important and the LDK-included chain::Listen
implementations are Send + Sync, this seems like an acceptable
tradeoff.

CC @johncantrell97

[TheBlueMatt]

Tagging 106 as it addresses a user issue.

[Kixunil]

Note that it seems to be possible to allow both Send + Sync and non-Send + Sync. Disclaimer: randomly noticed; only saw part of the code, I may be missing something.

// this blanket impl is backwards-incompatible but shouldn't be a huge issue for implementors
// it'll probably remove their code if it causes failure and DynamicWrapper shouldn't be needed any longer
impl<T: Listen + ?Sized> Listen for &T {
    fn block_connected(&self, block: &Block, height: u32) {
        (*self).block_connected(block, height)
    }
    fn block_disconnected(&self, header: &BlockHeader, height: u32) {
        (*self).block_disconnected(header, height)
    }
}

struct ChainListenerSet<T: Listen>(Vec<(u32, T)>);

The users can then pick whatever is the most suitable for them. Mock could just be Send + Sync or just have two mocks.

[TheBlueMatt]

Yea, removing the dyn would fix it, but sadly this API is intended to be used with at least two different types of Listen in the same Vec, hence the dyn :/

[Kixunil]

@TheBlueMatt you can still pass in Vec<&dyn Listen> or Vec<&(dyn Listen + Send + Sync)> depending on what's needed, that's the point. It only requires* the blanket impl above, which is backwards-incompatible with very little annoyance.

*Not strictly required, you could also write T: Listen + ?Sized bound and Vec<&T> but I find it messy. OTOH it'd enable use of default argument which could be nice if most people use dyn Listen + Send + Sync

[TheBlueMatt]

Oh! I see your point - &dyn Listen implements Listen + ?Sized so we can make it generic anyway, good point, indeed, can do that. The above blanket impl I'm not gonna bother with, though, cause users are ~always gonna need to as &dyn Listen here anyway, so just gonna stick with that.

[Kixunil]

&dyn Listen only implements Listen with the blanket impl. But dyn Listen does, so if you force reference you can avoid blanket impl - oh, now I see you already did the correct thing, so you understood it well. :)

[codecov-commenter]

Codecov Report

Merging #1349 (84a08db) into main (6259e7a) will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1349      +/-   ##
==========================================
+ Coverage   90.60%   90.80%   +0.19%     
==========================================
  Files          72       72              
  Lines       40075    42338    +2263     
==========================================
+ Hits        36310    38443    +2133     
- Misses       3765     3895     +130     

Impacted Files	Coverage Δ
lightning-block-sync/src/init.rs	95.80% <100.00%> (+2.24%)	⬆️
lightning/src/ln/functional_tests.rs	96.76% <0.00%> (-0.37%)	⬇️
lightning/src/routing/router.rs	92.03% <0.00%> (-0.07%)	⬇️
lightning/src/routing/scoring.rs	95.90% <0.00%> (+0.87%)	⬆️
lightning/src/ln/channelmanager.rs	86.13% <0.00%> (+1.43%)	⬆️
lightning/src/util/test_utils.rs	84.61% <0.00%> (+2.17%)	⬆️
lightning-invoice/src/utils.rs	92.76% <0.00%> (+4.62%)	⬆️

---

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 6259e7a...84a08db. Read the comment docs.