Update `ChannelConfig` of open channels #1050

luckysori · 2023-08-02T03:41:38Z

Fixes #952.

It was a good idea to rename the `user_config` in ecc14ce, because the original name was not very descriptive. But this `UserConfig` is just the LDK config in the context of 10101. So we should call it `ldk_config`.

Calling it `payments.rs` is better since we want to add more tests related to payments through a JIT channel.

In patch 2ddbee0 we mentioned that we wanted to be able to update the `ChannelConfig` for open channels (specifically to be able to update the routing fees charged).

holzeis

Nice, what parts of the ldk config can we change with that specifically? I guess that will have no impact on the handshake?

holzeis · 2023-08-02T08:41:21Z

crates/ln-dlc-node/src/node/mod.rs

@@ -770,6 +771,29 @@ where
            settings,
        })
    }
+
+    pub fn update_ldk_settings(&self, ldk_config: UserConfig) {


🤔 shouldn't we differentiate between public and private channels here? Otherwise we will always have on or the other if that gets executed.

Good question! What this PR adds shouldn't have an effect on whether channels are public or private because that is controlled by ChannelHandshakeConfig::announced_channel which is not controlled by ChannelConfig. In general, I'm not sure there is a mechanism to turn a private channel into a public one.

In relation to updating the UserConfig in general, we already have a default value for announced_channel for both coordinator and app. In the case of the coordinator we default to public channels and only override this when opening JIT channels in the event handler.

In theory we could change this default value with this API, but that would be weird. It's actually currently impossible to change it via the coordinator settings, which only support updating the forwarding_fee_proportional_millionths value:

10101/coordinator/src/settings.rs

Lines 99 to 110 in 2e574bf

/// The part of the coordinator settings pertaining to the LDK node.

pub fn to_ldk_settings(&self) -> UserConfig {

// Since we currently have to keep the coordinator settings in sync with the tests in

// `ln-dlc-node`, we let the library define the default settings (which is bad)

let mut ldk_config = ln_dlc_node::ldk_coordinator_config();

ldk_config

.channel_config

.forwarding_fee_proportional_millionths = self.forwarding_fee_proportional_millionths;

ldk_config

}

bonomat

Nice

luckysori · 2023-08-02T09:42:10Z

Nice, what parts of the ldk config can we change with that specifically?

With this PR the only thing that we are changing is the ability to override the ChannelConfig for open channels. Since this feature is only used by the coordinator and the coordinator currently only cares about updating the proportional routing fees, that's all that we can do.

Eventually we might want to use this feature to change other bits.

I guess that will have no impact on the handshake?

Yep, it will have no effect.

luckysori added 3 commits August 2, 2023 13:40

chore: Rename channel_config to ldk_config

c193401

It was a good idea to rename the `user_config` in ecc14ce, because the original name was not very descriptive. But this `UserConfig` is just the LDK config in the context of 10101. So we should call it `ldk_config`.

chore(ln-dlc-node): Rename multiple_payments test module

c63a776

Calling it `payments.rs` is better since we want to add more tests related to payments through a JIT channel.

feat: Update ChannelConfig of open channels

ea24f8a

In patch 2ddbee0 we mentioned that we wanted to be able to update the `ChannelConfig` for open channels (specifically to be able to update the routing fees charged).

luckysori requested review from bonomat and klochowicz August 2, 2023 03:41

luckysori self-assigned this Aug 2, 2023

luckysori linked an issue Aug 2, 2023 that may be closed by this pull request

Update proportional routing fee for existing channels #952

Closed

holzeis reviewed Aug 2, 2023

View reviewed changes

bonomat approved these changes Aug 2, 2023

View reviewed changes

holzeis approved these changes Aug 2, 2023

View reviewed changes

Base automatically changed from fix/dprint-rustfmt-nightly to main August 2, 2023 09:57

luckysori added this pull request to the merge queue Aug 2, 2023

Merged via the queue into main with commit e9dfc4a Aug 2, 2023
7 checks passed

luckysori deleted the feat/reconfigure-channels-after-update branch August 2, 2023 10:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `ChannelConfig` of open channels #1050

Update `ChannelConfig` of open channels #1050

luckysori commented Aug 2, 2023

holzeis left a comment

holzeis Aug 2, 2023

luckysori Aug 2, 2023

luckysori Aug 2, 2023

bonomat left a comment

luckysori commented Aug 2, 2023 •

edited

Loading

	/// The part of the coordinator settings pertaining to the LDK node.
	pub fn to_ldk_settings(&self) -> UserConfig {
	// Since we currently have to keep the coordinator settings in sync with the tests in
	// `ln-dlc-node`, we let the library define the default settings (which is bad)
	let mut ldk_config = ln_dlc_node::ldk_coordinator_config();

	ldk_config
	.channel_config
	.forwarding_fee_proportional_millionths = self.forwarding_fee_proportional_millionths;

	ldk_config
	}

Update ChannelConfig of open channels #1050

Update ChannelConfig of open channels #1050

Conversation

luckysori commented Aug 2, 2023

holzeis left a comment

Choose a reason for hiding this comment

holzeis Aug 2, 2023

Choose a reason for hiding this comment

luckysori Aug 2, 2023

Choose a reason for hiding this comment

luckysori Aug 2, 2023

Choose a reason for hiding this comment

bonomat left a comment

Choose a reason for hiding this comment

luckysori commented Aug 2, 2023 • edited Loading

Update `ChannelConfig` of open channels #1050

Update `ChannelConfig` of open channels #1050

luckysori commented Aug 2, 2023 •

edited

Loading