Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use SimpleArcPeerManager-GossipVerifier combo due to circular type dependency #2813

Closed
optout21 opened this issue Jan 4, 2024 · 1 comment · Fixed by #2822
Closed
Milestone

Comments

@optout21
Copy link
Contributor

optout21 commented Jan 4, 2024

I ran into this while trying to bump LDK to v0.0.119 in ldk-sample

PR #2773 simplified GossipVerifier, that takes now a APeerManager template parameter (instead of a few ones used in the peer manager). However, in ldk-sample, a SimpleArcPeerManager is used, that takes a GossipVerifier, leading to a circular type dependency.

Reverting #2773 would solve this, but Options:

Currently this blocks bumping LDK to latest in ldk-sample. See note: lightningdevkit/ldk-sample#126 (comment)

@optout21
Copy link
Contributor Author

optout21 commented Jan 4, 2024

Some more background:

The following line has been changed:

pub struct GossipVerifier<S: FutureSpawner,
...
{
    source: Blocks,
    peer_manager: Arc<PeerManager<Descriptor, CM, Arc<P2PGossipSync<Arc<NetworkGraph<L>>, Self, L>>, OM, L, CMH, NS>>,

to

    peer_manager: APM,

that is, a recursive reference was removed from GossipVerifier. In case the used peer manager indeed uses a RoutingMessageHandler that uses the same GossipVerifier, this recursive reference cannot be expressed in the type parameters as type declarations have no way to be recursive.

@tnull tnull added this to the 0.0.120 milestone Jan 10, 2024
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Jan 10, 2024
In 6765967 we relaxed the bounds
set on `UtxoLookup` to enable those using `RoutingMessageHandler`
other than `P2PGossipSync` to use `UtxoLookup`. Sadly, because this
requires having a concrete `PeerManager` type which does *not* use
`UtxoLookup` in the `RoutingMessageHandler` type, this broke users
who were directly using `P2PGossipSync`.

We could split `UtxoLookup` into two, with different bounds, for
the two use-cases, but instead here we simply switch to storing a
reference to the `PeerManager` via a `dyn Fn` which allows us to
wake the `PeerManager` when we need to.

Fixes lightningdevkit#2813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants