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

Add implementation for UpdateChannelConfig Api. #16

Merged
merged 2 commits into from
Oct 22, 2024

Conversation

G8XSU
Copy link
Contributor

@G8XSU G8XSU commented Oct 15, 2024

Based on #15

@G8XSU G8XSU mentioned this pull request Oct 15, 2024
13 tasks
@G8XSU G8XSU changed the title [WIP] Add implementation for UpdateChannelConfig Api. Add implementation for UpdateChannelConfig Api. Oct 15, 2024
@G8XSU G8XSU requested a review from jkczyz October 15, 2024 20:40
server/src/api/update_channel_config.rs Outdated Show resolved Hide resolved
server/src/api/update_channel_config.rs Outdated Show resolved Hide resolved
server/src/api/update_channel_config.rs Outdated Show resolved Hide resolved
server/src/api/update_channel_config.rs Outdated Show resolved Hide resolved
@G8XSU
Copy link
Contributor Author

G8XSU commented Oct 21, 2024

Addressed comments, rebased on top of #18 and #15.

@G8XSU G8XSU requested a review from jkczyz October 21, 2024 22:52
Comment on lines +17 to +23
//FIXME: Use ldk/ldk-node's partial config update api.
let current_config = node
.list_channels()
.into_iter()
.find(|c| c.user_channel_id.0 == user_channel_id)
.ok_or_else(|| ldk_node::NodeError::InvalidChannelId)?
.config;
Copy link

Choose a reason for hiding this comment

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

Should we do this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Afaiu, Currently there is no api in ldk-node for partial config update, this will have to wait until next ldk-node release.

Since the functionality/user-interface here is equivalent, I don't think there is much point in waiting till then.
Created: lightningdevkit/ldk-node#385

We can remove this once that is complete.

Copy link

Choose a reason for hiding this comment

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

Ah, right!

@G8XSU G8XSU requested a review from jkczyz October 21, 2024 23:30
@G8XSU G8XSU merged commit 9ce78ae into lightningdevkit:main Oct 22, 2024
1 check passed
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.

2 participants