[bug]: fees default to 253sat/kw if fees cannot be downloaded from feeurl about lnd HOT 5 OPEN

This caused all our LDK nodes who had channels with our LSP (which runs LND) to be force closed. This is a catastrophic bug, I hope a fix is prioritized.

2024-05-10 18:38:55 ERROR [lightning::ln::channelmanager:7150] Closing channel 04259a5524219241061608caa6216811cfcfcf713291fc3c839dde1d47a90a72 due to close-required error: Peer's feerate much too low. Actual: 253. Our expected lower limit: 2993
2024-05-10 18:38:55 ERROR [lightning::ln::channelmanager:8971] Force-closing channel: Peer's feerate much too low. Actual: 253. Our expected lower limit: 2993
2024-05-10 18:38:55 DEBUG [lightning::ln::channelmanager:2893] Finishing closure of channel due to Channel closed because of an exception: Peer's feerate much too low. Actual: 253. Our expected lower limit: 2993 with 0 HTLCs to fail
2024-05-10 18:38:55 INFO  [lightning::chain::channelmonitor:2796] Applying force close update to monitor 04259a5524219241061608caa6216811cfcfcf713291fc3c839dde1d47a90a72 with 1 change(s).

from lnd.

I think we should return the error here to be handled by the callers instead of returning the fee floor,

Looking at other implementations of EstimateFeePerKW, think we should do the same - upper systems need to know there's an error in fee estimation instead of defaulting to the default fee rate floor.

from lnd.

We should also smooth/clamp the updates as well. This way we avoid adjusting too sharply in either direction.

from lnd.

We should also smooth/clamp the updates as well. This way we avoid adjusting too sharply in either direction.

I wonder what's the best strategy to clamp the fee-rate, percentage of the current feerate seems to be not the best design when values are small hmm ?

Moreover maybe we should unify the behaviour for example for the bitcoind estimator we will return the fallback fee in case there is an error while fetching the data.

As @yyforyongyu said, best thing in case of error is to accept what the other party suggests. And in case the other party doesn't provide any value, just use the latest ones retrived.

In case of sudden change but no error, yes, it is probably better to clamp it over the course of, for example, 6 blocks.

For any of the two cases, you would need to store the information of the latest feeurl retrived, probably in a small separate file.

You would also need to return an error if too much time has passed since the last retrival happened. Again, I would suggest returning an error after 6 blocks without retriving anything. Also an error should be provided just at the beginning, exiting the app, if the first retrival fails and the file with the stored values does not exist or is too old.

from lnd.

We should also smooth/clamp the updates as well. This way we avoid adjusting too sharply in either direction.

I wonder what's the best strategy to clamp the fee-rate, percentage of the current feerate seems to be not the best design when values are small hmm ?

Moreover maybe we should unify the behaviour for example for the bitcoind estimator we will return the fallback fee in case there is an error while fetching the data.

from lnd.