Allow configuring different max fee rate from peer

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660

[benthecarman]

We've run into issues a few times where channels will get force closed because CLN's fee estimator isn't as good as ours so the fee rate will be too high, resulting in a force close. It'd be nice if we'd have the ability to be able to increase this limit because generally this isn't malicious, just different views of the mempool.

Alternatively would be nice if we can have an option to just deactivate this channel instead of closing it

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (989304e) 89.02% compared to head (0182349) 88.95%.
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@## main #2643 +/- ##
==========================================
- Coverage 89.02% 88.95% -0.07% 
==========================================
Files 112 112 Lines 87168 87224 +56 Branches 87168 87224 +56 ==========================================
- Hits 77605 77594 -11 - Misses 7327 7389 +62 - Partials 2236 2241 +5 

Files	Coverage Δ
lightning/src/ln/channel.rs	88.37% <88.88%> (+<0.01%)	⬆️
lightning/src/util/config.rs	65.55% <37.50%> (-2.33%)	⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

This API is a bit awkward cause it doesn't apply to anchor channels, and no limit will be applied to anchor channels at all once Bitcoin Core ships package relay. More generally - I'm kinda surprised you hit this? If you're regularly seeing a peer set a feerate we think is too high we can probably just increase the default limit somewhat - like you say its generally not malicious, though there is risk if we let it float too high.

[benthecarman]

yeah we had this happen to a few users, i guess our high priority fee rate was between 1-2 sats/vbyte. We since updated it to be a 1 block target instead of 3. But would be nice to be able to just jack this number up so we know this won't happen

[AnthonyRonning]

Still happening too unfortunately. Bitcoin core has very terrible fee rate estimates that don't reflect reality.

Does LDK use the high priority rate for anything else? I guess we're going to have to either fix CLN or crank up the range that LDK will allow. It's not acceptable to have massive force closures amongst all of our users whenever fee rates temporarily have a disagreement. That should be another issue in itself.

[benthecarman]

For added context, we're using mempool.space's fastest fee target for our High Priority and we had a user get this error this morning.

Channel closed because of an exception: Peer's feerate much too high. Actual: 11001. Expected upper limit: 6250

Right now our only way to prevent every user getting force closed is just jacking up our HighPriority fee target

[wpaulino]

Increasing your HighPriority fee rates will also affect all HTLC claims. This PR isn't actually allowing you to configure anything yet though? An alternative would be to expose a config option (that only applies to pre-anchors channels) allowing you to set max_counterparty_selected_feerate, the highest fee you'll accept.

[benthecarman]

This PR isn't actually allowing you to configure anything yet though?

You can just overwrite the function when you implement the trait, the current implementation will just be the default if the user doesn't implement it.

[benthecarman]

An alternative would be to expose a config option

Then we can't have it changing on the mempool, would just be a hard coded value.

[wpaulino]

We could do a multiplier on the queried HighPriority fee rate?

[benthecarman]

We could do a multiplier on the queried HighPriority fee rate?

That could work

[benthecarman]

Changed to be a part of ChannelConfig, not sure if I handled the serialization correctly however

[TheBlueMatt]

I'd very much rather do #2659 over adding more config settings and more indirection.

[benthecarman]

Closing in favor of #2660