Skip to content

feat: channel events#193

Open
frnandu wants to merge 1 commit into
lightningdevkit:mainfrom
frnandu:feat/channel-events
Open

feat: channel events#193
frnandu wants to merge 1 commit into
lightningdevkit:mainfrom
frnandu:feat/channel-events

Conversation

@frnandu
Copy link
Copy Markdown

@frnandu frnandu commented Apr 17, 2026

fixes #134

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 17, 2026 •
edited
Loading

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Anyitechs
Anyitechs reviewed Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Anyitechs Anyitechs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this. Please fix CI if this is ready for review (The proto files are now out of date and needs to be updated, and the changes introduced needs to be formatted too)

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 20, 2026

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 22, 2026

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman
benthecarman requested changes Apr 22, 2026
View reviewed changes
Comment thread ldk-server/src/main.rs Outdated
Comment thread ldk-server/src/main.rs Outdated
Comment thread ldk-server/src/main.rs Outdated
Comment thread e2e-tests/tests/e2e.rs
Anyitechs
Anyitechs reviewed Apr 22, 2026
View reviewed changes
Comment thread ldk-server/src/main.rs
mod service;
mod util;

use std::collections::HashSet;
Copy link
Copy Markdown
Contributor

@Anyitechs Anyitechs Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to use the HashSet from HashBrown here instead, since it's already in the tree?

Copy link
Copy Markdown
Author

@frnandu frnandu May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switching would add a new explicit dependency for little benefit in this case. std::collections::HashSet is the idiomatic default and already optimized AFAIK

@benthecarman
Copy link
Copy Markdown
Collaborator

benthecarman commented May 14, 2026

something weird happened when you merged main, can you just rebase your commit on main?

@frnandu frnandu force-pushed the feat/channel-events branch from a4765ad to d7981f0 Compare May 14, 2026 19:58
@frnandu frnandu requested a review from benthecarman May 14, 2026 20:20
@benthecarman
Copy link
Copy Markdown
Collaborator

benthecarman commented May 14, 2026

commit structure is still weird here. should just be the one for the feature

@frnandu frnandu force-pushed the feat/channel-events branch from d7981f0 to 744f68f Compare May 14, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benthecarman benthecarman Awaiting requested review from benthecarman

1 more reviewer

@Anyitechs Anyitechs Anyitechs left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Channel state change event publishing to grpc subscriptions

4 participants

@frnandu @ldk-reviews-bot @benthecarman @Anyitechs