lnd icon indicating copy to clipboard operation
lnd copied to clipboard

lncli: `base_fee_msat` and `fee_rate_ppm` for `openchannel`

Open hieblmi opened this issue 2 years ago • 4 comments

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 https://github.com/lightningnetwork/lnd/pull/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

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

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

hieblmi avatar Jul 18 '22 22:07 hieblmi

@positiveblue I've addressed the issues you raised in the latest commit.

hieblmi avatar Aug 09 '22 03:08 hieblmi

@positiveblue: review reminder @hieblmi, remember to re-request review from reviewers when ready

lightninglabs-deploy avatar Aug 29 '22 22:08 lightninglabs-deploy

@sputn1ck @positiveblue I've added the deletion of the forwarding policy in the private channel funding flow: https://github.com/lightningnetwork/lnd/blob/a948ba91db697e14c9b7b26e26fee969c7202aa9/funding/manager.go#L3127

I've tested this change in two ways.

Firstly opened a private channel on testnet and verified the database entry with the boltbrowser:

lnclit openchannel --node_key 02340ceae1c0e1779577a947ff1b09df9962e20dccfad31772d437501c31eca28f --local_amt 20004 --base_fee_msat 3333 --fee_rate_ppm 3333 --private
{
	"funding_txid": "b2128cd17b867a34f3b5f394a06aa602c6ee3e6943da8534887bf6453cfffa9e"
}

Then checked the bolt channel.db with https://github.com/guggero/boltbrowser and see an entry for the forwarding policy: image Waited for 6 confirmations and checked the channel.db again. image

Secondly I added additional checks in manager_test.go to make sure that there are is no forwarding policies saved in the db at the end of each test including the two private channel tests which. https://github.com/lightningnetwork/lnd/blob/188ad1cad9a8116896d4d96e00795280875718e9/funding/manager_test.go#L1293

hieblmi avatar Sep 10 '22 16:09 hieblmi

Addressed nits in latest change set.

hieblmi avatar Sep 20 '22 00:09 hieblmi

@guggero addressed the nits in latest change set.

hieblmi avatar Sep 27 '22 12:09 hieblmi

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).

guggero avatar Sep 27 '22 13:09 guggero

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

hieblmi avatar Sep 27 '22 15:09 hieblmi

@guggero addressed latest nits.

hieblmi avatar Sep 28 '22 14:09 hieblmi

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

hieblmi avatar Sep 29 '22 14:09 hieblmi