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

CLN choosing absurd fee rates for cooperative closures #7242

Closed
benthecarman opened this issue Apr 19, 2024 · 11 comments · Fixed by #7252
Closed

CLN choosing absurd fee rates for cooperative closures #7242

benthecarman opened this issue Apr 19, 2024 · 11 comments · Fixed by #7252

Comments

@benthecarman
Copy link
Contributor

cc @gkrizek @TonyGiorgio

image

image

@rustyrussell
Copy link
Contributor

Is CLN the opener in this example? If we're the opener, we negotiate between min and max fees. If we're not the opener, we negotiate between min and entire channel capacity, because it's their funeral, right? And this looks a lot like that is the case here...

OR did you set "ignore fee limits" on the channel (or globally). That's the other case where max is unlimited...

@benthecarman
Copy link
Contributor Author

In all these cases CLN was the channel opener

@benthecarman
Copy link
Contributor Author

ignore-fee-limits is set, this is so we don't experience force closes, the docs do not say anything about this effecting coop closes

@gkrizek
Copy link
Contributor

gkrizek commented Apr 22, 2024

Like Ben said, ignore-fee-limit is set globally but all channels are opened by the CoreLN node (our side opens). So according to the docs I don't think it should have an effect.

@rustyrussell
Copy link
Contributor

If you open all the channels, then it should not effect force close rate? If it does, then I'm puzzled.

Docs indeed do not detail this, but one reason for the option initially was to enable mutual close when we don't agree on fees. But all it does is increase the maximum (and drop the minimum) we will accept: we still negotiate based on what they send us, starting at our ideal.

What was the actual fee rate on the final mutual close? Did it actually choose an absurd feerate, or just set no limits?

@rustyrussell
Copy link
Contributor

Note: for non-anchor channels, we could cap the max even in the "ignore-fee-limits" case to the fee for the final unilateral close. (Anchor channels lowball fees because they rely on the anchors to CPFP, so they don't set a maximum the same way).

@TonyGiorgio
Copy link
Contributor

TonyGiorgio commented Apr 22, 2024

I can try to find the logs again, on both sides.

In all cases, it was a non anchor coop close initiated by LDK while the CLN node was the opener. LDK proposed a wide open fee range, and CLN picked an insanely high fee, much higher than the minimum needed. LDK accepted and then the channel close was broadcasted.

@TonyGiorgio
Copy link
Contributor

If you open all the channels

That's not always the case.

@rustyrussell
Copy link
Contributor

I can try to find the logs again, on both sides.

In all cases, it was a non anchor coop close initiated by LDK while the CLN node was the opener. LDK proposed a wide open fee range, and CLN picked an insanely high fee, much higher than the minimum needed. LDK accepted and then the channel close was broadcasted.

Pretty sure LDK supports fee range, so this is what would have happened:

  1. CLN proposes 1368 sats, with an acceptable range of 183 to a bazillion.
  2. LDK didn't like that, so proposed a different number (and range).
  3. That number was (of course) acceptable to CLN's wide range, so it agreed.

@rustyrussell
Copy link
Contributor

OK, @TheBlueMatt confirms: LDK picks the upper bound. The spec, however, is pretty clear here:

  - otherwise, if the receiver agrees with the fee:
    - SHOULD reply with a `closing_signed` with the same `fee_satoshis` value.
  - otherwise:
    - MUST propose a value "strictly between" the received `fee_satoshis` and its previously-sent `fee_satoshis`.

i.e. you should reply with the same number they gave, unless it's unacceptable.

With this in the wild, I think this option is now unexpectedly dangerous. I will disable it for close.

rustyrussell added a commit to rustyrussell/lightning that referenced this issue Apr 23, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
@TonyGiorgio
Copy link
Contributor

Here's one of the logs on the LDK side. I think your description above sounds correct. Thanks for tracking that down!

1eec 2024-04-05 04:24:04.045 DEBUG [lightning::ln::channelmanager:7113] Received channel_update ChannelUpdate { signature: 30440220613c9bad5f8e0145a328b9a7d24402e4a78fe6319e3eb69710385e648dbd98f202202f7e66bf6c1cecc99e766622fcb6b4be4d0d185be87973772221213e35837ac9, contents: UnsignedChannelUpdate { chain_hash: 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000, short_channel_id: 891735815969439744, timestamp: 1712291044, flags: 2, cltv_expiry_delta: 34, htlc_minimum_msat: 1, htlc_maximum_msat: 333,001,000, fee_base_msat: 1000, fee_proportional_millionths: 10, excess_data: [] } } for channel efd5b0cd9903a7d43f0f3b6eac2e9344beb097cfeed26b1a2ea6ddc5fa4321cb.
1eec 2024-04-05 04:24:04.045 DEBUG [lightning::ln::peer_handler:1517] Error handling message from 03aefa43fbb4009b21a4129d05953974b7dbabbbfb511921410080860fca8ee1f0; ignoring: Couldn't find channel for update
1eec 2024-04-05 04:24:04.102 TRACE [mutiny_core::networking::socket:62] received binary data from websocket: 172
1eec 2024-04-05 04:24:04.103 DEBUG [lightning::ln::channelmanager:7113] Received channel_update ChannelUpdate { signature: 30440220035c1135450580eef06218327a7a9bdb3207a2457f37179b3c9d4c6ca1594f350220297c2592d766ac00bbd123080c84e0c31de429b26a40d8f1d93f9602a6064f4b, contents: UnsignedChannelUpdate { chain_hash: 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000, short_channel_id: 891735815969439744, timestamp: 1712291045, flags: 2, cltv_expiry_delta: 34, htlc_minimum_msat: 1, htlc_maximum_msat: 333001000, fee_base_msat: 1000, fee_proportional_millionths: 10, excess_data: [] } } for channel efd5b0cd9903a7d43f0f3b6eac2e9344beb097cfeed26b1a2ea6ddc5fa4321cb.
1eec 2024-04-05 04:24:04.103 DEBUG [lightning::ln::peer_handler:1517] Error handling message from 03aefa43fbb4009b21a4129d05953974b7dbabbbfb511921410080860fca8ee1f0; ignoring: Couldn't find channel for update
1eec 2024-04-05 04:24:04.536 TRACE [mutiny_core::networking::socket:62] received binary data from websocket: 158
1eec 2024-04-05 04:24:04.536 TRACE [lightning::ln::peer_handler:1632] Received message ClosingSigned(ClosingSigned { channel_id: ChannelId([239, 213, 176, 205, 153, 3, 167, 212, 63, 15, 59, 110, 172, 46, 147, 68, 190, 176, 151, 207, 238, 210, 107, 26, 46, 166, 221, 197, 250, 67, 33, 203]), fee_satoshis: 1545, signature: 304402207c8f77ea1bb29be8d717bdaf29af2018fcc054db4b1ee0ebb88169d65b124d7102200fc369308b85f56ddb01f3387e7445f96e92c208864e820f386dd9d4f0e47ab0, fee_range: Some(ClosingSignedFeeRange { min_fee_satoshis: 195, max_fee_satoshis: 3315714751 }) }) from 03aefa43fbb4009b21a4129d05953974b7dbabbbfb511921410080860fca8ee1f0
1eec 2024-04-05 04:24:04.539 TRACE [mutiny_core::fees:177] Got fee rate from saved cache!
1eec 2024-04-05 04:24:04.539 TRACE [mutiny_core::fees:177] Got fee rate from saved cache!
1eec 2024-04-05 04:24:04.542 DEBUG [lightning::ln::peer_handler:2148] Handling SendClosingSigned event in peer_handler for node 03aefa43fbb4009b21a4129d05953974b7dbabbbfb511921410080860fca8ee1f0 for channel efd5b0cd9903a7d43f0f3b6eac2e9344beb097cfeed26b1a2ea6ddc5fa4321cb
1eec 2024-04-05 04:24:04.542 TRACE [lightning::ln::peer_handler:1279] Enqueueing message ClosingSigned { channel_id: ChannelId([239, 213, 176, 205, 153, 3, 167, 212, 63, 15, 59, 110, 172, 46, 147, 68, 190, 176, 151, 207, 238, 210, 107, 26, 46, 166, 221, 197, 250, 67, 33, 203]), fee_satoshis: 212482, signature: 304402203536762e74ec8b88a8d28473a3aad310a60f2e5592de6aa1a560aa5f3f7020300220453071d490d99cdf07517f3d61bfaf82e23e7d385690e6a83f0f911b70bdfd74, fee_range: Some(ClosingSignedFeeRange { min_fee_satoshis: 1347, max_fee_satoshis: 212482 }) } to 03aefa43fbb4009b21a4129d05953974b7dbabbbfb511921410080860fca8ee1f0
1eec 2024-04-05 04:24:04.542 TRACE [lightning_background_processor:693] Persisting ChannelManager...
1eec 2024-04-05 04:24:04.543 TRACE [lightning_background_processor:693] Done persisting ChannelManager.
1eec 2024-04-05 04:24:04.544 TRACE [mutiny_core::networking::proxy:92] sent data down websocket
1eec 2024-04-05 04:24:04.625 TRACE [mutiny_core::networking::socket:62] received binary data from websocket: 158
1eec 2024-04-05 04:24:04.625 TRACE [lightning::ln::peer_handler:1632] Received message ClosingSigned(ClosingSigned { channel_id: ChannelId([239, 213, 176, 205, 153, 3, 167, 212, 63, 15, 59, 110, 172, 46, 147, 68, 190, 176, 151, 207, 238, 210, 107, 26, 46, 166, 221, 197, 250, 67, 33, 203]), fee_satoshis: 212482, signature: 30440220511689f49510de1c7903f74bb3494bcd870701d581a18f57c568c017b9a35d0102200b1a91d6a2488cca0feff39d2f15e36e67769bf9a28204b0aaefe9cbe07b0d2c, fee_range: Some(ClosingSignedFeeRange { min_fee_satoshis: 195, max_fee_satoshis: 3315714751 }) }) from 03aefa43fbb4009b21a4129d05953974b7dbabbbfb511921410080860fca8ee1f0
1eec 2024-04-05 04:24:04.628 INFO  [lightning::ln::channelmanager:6613] Broadcasting closing tx with txid fb03bbbaaff48111e2e940db305068e65d216b7b2e18789839e212aedffb36da

rustyrussell added a commit to rustyrussell/lightning that referenced this issue Apr 23, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
rustyrussell added a commit to rustyrussell/lightning that referenced this issue May 7, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
rustyrussell added a commit to rustyrussell/lightning that referenced this issue May 9, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
rustyrussell added a commit to rustyrussell/lightning that referenced this issue May 21, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
rustyrussell added a commit to rustyrussell/lightning that referenced this issue May 22, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
rustyrussell added a commit to rustyrussell/lightning that referenced this issue May 22, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
rustyrussell added a commit to rustyrussell/lightning that referenced this issue May 22, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#7242
endothermicdev pushed a commit that referenced this issue May 22, 2024
LDK will pick the *upper* limit (see: lightningdevkit/rust-lightning#3014)!

It should not do this, but since you can set a manual range for mutual close, it's probably better to disable this option for close, as it's even more dangerous than documented.

Changelog-Changed: config/JSON: --ignore-fee-limits / setchannel ignorefeelimits no longer applies to mutual close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #7242
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants