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

Trivial PeerManager cleanups #2249

Merged

Conversation

TheBlueMatt
Copy link
Collaborator

Two rather small PeerManager and downstream things type cleanups. I originally wrote the second to make #2248 cleaner, but it actually can't be used for that as it leads to circular type definitions.

@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2023

Codecov Report

Patch coverage: 86.66% and project coverage change: -0.01 ⚠️

Comparison is base (0e8da58) 91.59% compared to head (b65e389) 91.59%.

❗ Current head b65e389 differs from pull request most recent head 8ec1ddf. Consider uploading reports for the commit 8ec1ddf to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2249      +/-   ##
==========================================
- Coverage   91.59%   91.59%   -0.01%     
==========================================
  Files         104      104              
  Lines       51976    51981       +5     
  Branches    51976    51981       +5     
==========================================
+ Hits        47609    47613       +4     
- Misses       4367     4368       +1     
Impacted Files Coverage Δ
lightning-background-processor/src/lib.rs 83.18% <75.00%> (-0.12%) ⬇️
lightning-net-tokio/src/lib.rs 78.61% <85.00%> (+0.20%) ⬆️
lightning/src/ln/peer_handler.rs 62.57% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

`PeerManager` takes a `MessageHandler` struct which contains all
the known message handlers for it to pass messages to. It then,
separately, takes a `CustomMessageHandler`. This makes no sense, we
should simply include the `CustomMessageHandler` in the
`MessageHandler` struct for consistency.
@TheBlueMatt TheBlueMatt force-pushed the 2023-04-less-pm-bounds branch from b65e389 to 8ec1ddf Compare April 30, 2023 04:04
@wpaulino
Copy link
Contributor

wpaulino commented May 2, 2023

LGTM after squash

A while back, in tests, we added a `AChannelManager` trait, which
is implemented for all `ChannelManager`s, and can be used as a
bound when we need a `ChannelManager`, rather than having to
duplicate all the bounds of `ChannelManager` everywhere.

Here we do the same thing for `PeerManager`, but make it public and
use it to clean up `lightning-net-tokio` and
`lightning-background-processor`.

We should likely do the same for `AChannelManager`, but that's left
as a followup.
@TheBlueMatt TheBlueMatt force-pushed the 2023-04-less-pm-bounds branch from c47bafb to 14c6810 Compare May 2, 2023 22:17
@TheBlueMatt
Copy link
Collaborator Author

done.

@TheBlueMatt TheBlueMatt merged commit ec3de62 into lightningdevkit:main May 3, 2023
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 this pull request may close these issues.

4 participants