lncli: base_fee_msat and fee_rate_ppm for openchannel

[hieblmi]

Motivation

Node operators often see their newly confirmed channels' local balance leaking to the remote side especially when connecting to capacity sinks while having configured low default fee rates in lnd.conf, e.g.

• https://twitter.com/bitcoinftm/status/1542083458796748800.
• https://twitter.com/callebtc/status/1556985524245729283

The issue is also mentioned here #6610.

This PR introduces two parameters base_fee_msat and fee_rate for lncli openchannel in order to default to the so specified fees in the channel announcement phase. This way immediate channel leakage can be avoided.

Implementation

1. Four fields are introduced to OpenChannelRequest, baseFee, feeRate, useBaseFee and useFeeRate. The use* fields are added to distinguish between the scenarios where the user provides fee values base_fee 0 / fee_rate 0 on lncli openchannel and where the user omits one of the fee values and when the default config parameters for fees should be used in the channel announcement.
2. The rpcserver.go parses the fields into initFundingMsg
3. Within funding/manager.go these new fields are added to a channel reservation context.
4. When the FundingSigned stage is reached the reservation context is deleted so before that we store the fields per channel id in a new database bucket. Once the channel announcement stage is reached we read the channel fees from the database into the channel announcement structure and delete the fees from the database.
5. In newChanAnnouncement we check if we find a fee entry in the database for a completed channel id. If so we apply the fees provided and delete the entry from the bucket.

Steps to Test

1. See attached testOpenChannelUpdateFeePolicy and funding/manager_test.go
2. Tested differrent scenarios on testnet while having default fees configured in lnd.conf.

lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40000
lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40001 --base_fee_msat 9999
lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40002 --fee_rate_ppm 9999
lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40003 --base_fee_msat 9999 --fee_rate_ppm 9999
lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40004 --base_fee_msat 0 --fee_rate_ppm 9999
lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40004 --base_fee_msat 9999 --fee_rate_ppm 0
lnclit openchannel --node_key 030425d8babe3ab6dfc065e69dd8b10ce6738c86ea7d634324c913e21620fa5eaf --local_amt 40004 --base_fee_msat 0 --fee_rate_ppm 0

Pull Request Checklist

Testing

• Your PR passes all CI checks.
• Tests covering the positive and negative (error paths) are included.

Code Style and Documentation

• The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• Commits follow the Ideal Git Commit Structure.
• There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

[positiveblue]

Nice work @hieblmi this looks pretty good!

I left some comments/nits.

The most pressing thing would be to ensure that this data is persisted + loaded when lnd starts again so we don't lose it during restarts.

Something useful could also be break the PR in smaller commits so they are easier to review:

• Add fields to support custom fees in funding
• Add rpc fields
• Update rpc server to use the new fields
• Add new fields to cmd
• Add Notes

[SachinMeier]

This is a great change! Excited to see it merged.

[hieblmi]

@guggero addressed the nits in latest change set.

[guggero]

Thanks for addressing the nits in all commits, not just the ones I pointed out. Though the formatting is off, please see https://github.com/lightningnetwork/lnd/blob/master/docs/code_formatting_rules.md (part of the checklist in the PR template).

[hieblmi]

@guggero I think I caught all formatting issues and will keep them in mind.

[guggero]

Last set of nits, then we're good to go 🚀

[hieblmi]

@guggero addressed latest nits.

[guggero]

I thought I already added this comment in my last review, but it looks like I forgot. But those struct fields all need an individual Godoc comment.

[guggero]

The question about the TODO is still open. Is this something that actually needs to be decided or can we remove the TODO instead?
Also, the lines following after the comment are now duplicated between this case and the default case. If we set the values when we initialize chanUpdateAnn and only overwrite what we can actually configure this switch statement becomes easier to read and understand IMO.

[guggero]

LGTM 🎉

[hieblmi]

Unreal 🍾🎉. Thanks yalls for reviewing and teaching this flow!