Limit FeeRate change for the UpdateFee msg to prevent sharp changes

[ziggie1984]

Relates to #7666 and #7756

Detailed Problem explanation:

In lnd (lightning) there exist the feeupdate msg which allows channel openers to change the fee of the commitment transaction. For STATIC_REMOTE_KEY channels HTLCs which are part of the Commitment Transaction do have Timeout/Success Transactions which are signed with the same feerate as the Commitment Transaction (see BOLT3 for more details about the HTLC Transaction). The FeeUpdate is only allowed to be sent by the Channel Opener but it should be in acceptable Limits, because especially for non-achor channels there is no way to accelerate this transaction (only if you pay a miner directly). CLN nodes will check for a specific range and will not allow the feeupdate from happening. If CLN does reject the feeupdate the channel will either be force-closed by the peer (lndk) or the channel becomes unusable and needs to be force-closed eventually by the peer.

This PR fixes 2 Problems:

No valid Fee Estimation by our backend

In case you either wipe your mempool or restart a new bitcoind node you want to connect to your lnd node, there is a short time period where bitcoind is not reporting any fee estimations because it does not have enough data available.

It will report:

bitcoin-cli estimatesmartfee 3
{
  "errors": [
    "Insufficient data or no feerate found"
  ],
  "blocks": 0
}

In such cases lnd would just read this response and the fee estimation would remain zero and then would just enforce the min mempool fee instead. This is a bad choice because when our mempool is big enough this fee rate is always 1 sat/vbyte and does not represent the current fee rate conditions. This PR will return an error in such cases and lnd will use its fallback feerate of 25 sats/vbyte. It is always better to overestimate rather than underestimate in such cases.

Limit Fee Estimation when channel is drained locally

Moreover Lnd reevaluates the MaxFeeRate it is willing to allocate for a Channel every time a feeupdate msg will be sent. When the channel is locally drained this could lead to a situation where we decrease the feerate all the way to the fee floor which does not reflect the current bitcoin network fee rate correctly and could also lead to our peer rejecting these low fee rate updates. With this PR we do not lower the MaxFeeRate we are willing to allocate for this channel if it would fall below the old fee rate. This guarantees that if we had a high fee rate on a channel, that we do not decrease it when the local balance is drained.

Locking for a Concept Ack here before writing tests.

This change will not fix existing channels which ran into this issue, but will most likely prevent future channels form being unusable when proposing very low fee rate updates leading to the channel-peer rejecting those.

Change Description

~~This PR limits the FeeRate update to a maximum of 30% of the old feerate. It only applies to non-anchor channels because for those the negotiated fee cannot be changed when resolving on-chain.~~

~~Open for discussions whether we want to pursue this route if 30% is too conservative and so one.~~

~~Needs some style fixing will solve this in the coming days.~~

Tests

I currently just updated the unit tests in the lnwallet package. I think it is enough and testing this in the htlcswitch where we drain the channel locally and send consecutively fee updates with the default fee-allocation might not be worth it IMO, what are your takes?

[ziggie1984]

~~This concept will not fix the problem, stated in https://github.com/lightningnetwork/lnd/issues/7756#issuecomment-1638969967. I need to rework this to make sure we do not scale down the comitment fee all the way to 253 sat/kw.~~

~~Hold on for review yet !~~

[kornpow]

Will this PR fix stuck channels or will it only prevent channels from entering a stuck state in the future?

[ziggie1984]

Will this PR fix stuck channels or will it only prevent channels from entering a stuck state in the future?

It cannot undo the feeupdate which happend in the past (your case currently) tho it will prevent low feeupdates in the future.

This change will not fix existing channels which ran into this issue, but will most likely prevent future channels form being unusable when proposing very low fee rate updates leading to the channel-peer rejecting those.

[ziggie1984]

Suffered from this behaviour on my local node as well now, I think this should be addressed asap, Channel is unusable now until we reach lower feelimits.

2023-12-10 13:30:59.882 [WRN] HSWC: ChannelLink(aa4a1c1313bed1063a9529840062c3263aced0b544b28105e79019f91e9c2bba:2): received warning message from peer: chan_id=ba2b9c1ef91990e70581b244b5d0ce3a26c362008429953a06d1be13131c4aa8, err=update_fee 253 outside range 4724-702240 (currently 1795)

[ziggie1984]

I think if we have https://github.com/lightningnetwork/lnd/pull/8096 we don't need this anymore since we would then have a fee buffer?

We would still need this, because the evaluation of the fee (we are willing to pay) would decrease over time and because the buffer is tied to the feerate we would, because of a feerate decrease, decrease the buffer => our channel would get drained more => our local balance shrinks again => the fee for the commitment would shrink as well in the next step => potential worst case fee-floor

[ziggie1984]

Looking good, just a few nits. One thing I think we should do in a followup PR is to make it more clear on how we update fees for anchor vs non-anchor channels here:

Agree I think this relates to https://github.com/lightningnetwork/lnd/issues/8302, which should get rid of the MaxAnchorsCommitFeeRate config setting and always make sure the relay feerate of its average Peers is met.

[kornpow]

🚢 would be great for this ship to set sail

[yyforyongyu]

need a second reviewer 👀 cc @saubyk

[lightninglabs-deploy]

@ziggie1984, remember to re-request review from reviewers when ready

[coderabbitai[bot]]

[!IMPORTANT]

Auto Review Skipped

Auto reviews are limited to the following labels: llm-review. Please add one of these labels to enable auto reviews.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Walkthrough

This update addresses several key aspects of fee management and policy updates in the Lightning Network Daemon (LND), including adjustments to fee estimation, channel fee policy handling, and the addition of Taproot witness types to RPC calls. It improves the reliability and specification compliance of fee handling during channel openings and commitment transactions, enhancing overall network stability and user experience.

Changes

Files	Change Summary
docs/release-notes/release-notes-0.18.0.md	Summarized release notes detailing fee management improvements and Taproot support.
lnwallet/chainfee/estimator.go	Adjustments for better fee estimation and logging.
lnwallet/channel.go .../channel_test.go	Modifications to fee rate calculations in channels and updates to corresponding tests.

Assessment against linked issues

Objective	Addressed	Explanation
Fee rate and policy handling improvements for channel operations [#7756, #8018, #8396]	✅
Addressing fee estimation discrepancies and backend setup issues [#8018]	❌	The changes focus on fee buffer adjustments and do not directly address backend setup discrepancies.
Ensuring UpdateChannelPolicy consistency and addressing failed payments due to fee issues [#8396]	❌	The release notes do not mention changes to UpdateChannelPolicy or payment failure issues directly.
Adding Taproot witness types to rpc [#7756]	✅

Related issues

• #7666: The adjustments in fee handling and policy updates may indirectly address interoperability issues between CLN and LND by ensuring more consistent fee rate proposals and updates, potentially mitigating the issue where Update_Fee messages are not accepted.

🎉🐇✨
In the fields of code, where logic threads weave,
A rabbit hopped forth, changes to conceive.
With a flick and a hop, fee woes take their leave,
Taproot in sight, as we aim to achieve.
Through tests and through trials, our goals interlace,
In the digital burrow, progress takes its place.
🌟🐾

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[ziggie1984]

@coderabbitai review

[ziggie1984]

Last Push was just a rebase.

[ProofOfKeags]

Idk what's going on with CI but there's a lot of complaining checks that I can't figure out why they are failing.

[guggero]

Related to the build cache. Going to put up a fix to reduce the cache size. @Crypt-iQ you self-requested review on this PR after 2 approvals. Should we block on your review?

[Crypt-iQ]

Related to the build cache. Going to put up a fix to reduce the cache size. @Crypt-iQ you self-requested review on this PR after 2 approvals. Should we block on your review?

Yes

[guggero]

Needs a rebase.