Add option to set remote channel reservation in open channel by roeierez · Pull Request #2708 · lightningnetwork/lnd

roeierez · 2019-02-26T19:54:17Z

This PR adds the option for a channel initiator to set the “channel_reserve_satoshis”, as defined in Bolt 2 for open_channel message.
Before this change, the field was fixed to 1% of the channel capacity (the recommended value in Bolt 2).
It seems reasonable to allow the channel initiator to change this value if it is willing to take the risk (or responsibility) of watching and monitoring the channel on-chain.
With the ability to dynamically set the channel reserve, nodes will be able to provide the other side better liquidity, enabling them to spend more funds

roeierez · 2019-07-12T21:09:03Z

Do you think it worth scheduling this PR for 0.7.1 release now that the channel reserve fields are in the RPC interface (#3238)?

halseth · 2019-07-18T12:20:18Z

Can target for 0.8 possibly.

roeierez · 2019-07-18T12:50:11Z

Can target for 0.8 possibly.

That would be great!

wpaulino · 2019-07-30T18:45:59Z

Needs a rebase.

roeierez · 2019-08-01T09:23:13Z

Rebased.

Roasbeef · 2019-08-07T02:29:42Z

With the ability to dynamically set the channel reserve

Likely just a choice of wording here, but isn't dynamic, even with this it would still be fixed. Also this isn't costless, by setting this value to zero or near zero (as I'm guessing you'd like to do), you introduce a costless attack for who ever you open a channel with.

roeierez · 2019-08-07T08:13:48Z

Likely just a choice of wording here, but isn't dynamic

Agree. Removed the word dynamically.

you introduce a costless attack for who ever you open a channel with

Right. To me, though, it looks like it is a business decision that shouldn't be limited by the protocol itself. Default value may remain 1%, but it is up to the initiator to change the value based on its business model. For example, Bitrefill offers their users to buy channels. In this case, users already have skin in the game.

cfromknecht

looks pretty good on first pass!

cfromknecht · 2019-08-07T23:39:03Z

fundingmanager.go

if i'm reading this correctly, we'll only use the user chosen reserve if it is at least the our dust limit. it doesn't seem obvious that it would be ignored if the value is too low. should we check this sooner in the pipeline and return an error?

I agree. I have shifted the input validation code to rpcserver.go similar to the way other rpc fields are being validated: https://github.com/lightningnetwork/lnd/pull/2708/files#diff-01ada5680aaa18fc79cbf78b2d55a37bR1366

wpaulino

LGTM, though we should add some tests at the funding manager level with different values (valid and invalid).

wpaulino · 2019-08-13T00:01:04Z

server.go

Nit: move next to remoteCsvDelay?

roeierez · 2019-08-13T08:18:34Z

we should add some tests at the funding manager level with different values (valid and invalid)

Sounds good! I will work on that.

roeierez · 2019-08-27T09:15:33Z

@wpaulino I added tests for:

custom valid value.
non valid value below dust.
non valid value over capacity.

For the functionality to be testable I needed to add validation for the channel reserve inside the fundingmanager.go.
Although I didn't see other parameters validated there, I think it is good to have it there since there are other code flows that use the fundingmanager besides RPC interface.
Also rebased.

halseth · 2020-09-24T09:07:36Z

This is no longer blocked by anchor outputs, and can be rebased.

This commit is the basis of enabling the intiator of a channel to determine the amount of satoshis he requires the remote peer to reserve as a direct payment in his side of the channel As stated in Bolt 2.

This commit uses the "remote_chan_reserve_sat" field value that was added to the OpenChannelRequest in the funding workflow. Upon start of the workflow the handleInitFundingMsg code was changed to take this value into account, save it in the pending channel reservation and finally read the value when creating the channel constraints for the remote contribution in handleFundingAccept.

This commit adds the lncli support to configure the remote channel reservation value.

This commit adds validation for channel reserve custom parameter in the funding manager itself. Since opening a channel is not done only from the RPC interface, this validation makes sure callers won't pass a wrong value. It also makes the channel reserve functionality testable.

This commit adds three test cases for the remote channel reserve parameter: 1. custom valid value. 2. non valid value bellow dust. 3. non valid value over capacity.

wpaulino · 2021-05-03T20:30:30Z

lnrpc/rpc.proto

    uint32 max_local_csv = 17;
+
+    // The number of satoshis we require the remote peer to reserve.
+    int64 remote_chan_reserve_sat = 18;


Nit: make uint64?

wpaulino · 2021-05-03T20:33:07Z

rpcserver.go

+	localFundingAmt btcutil.Amount) error {
+
+	dustLimit := lnwallet.DefaultDustLimit()
+	if chanReserve > 0 && chanReserve < dustLimit {


Nit: could omit the > 0 check if the proto type is unsigned.

wpaulino · 2021-05-03T20:39:42Z

funding/manager.go

 	if chanReserve == 0 {
 		chanReserve = f.cfg.RequiredRemoteChanReserve(capacity, ourDustLimit)
 	}
+	if chanReserve < ourDustLimit {


We already have a similar check in CommitConstraints, though that doesn't check against the capacity.

wpaulino · 2021-05-03T20:40:23Z

funding/manager_test.go

+		case <-alice.msgChan:
+			t.Fatalf("init funding workflow should fail with invalid chan"+
+				"reserve %v", chanReserve)
+		case <-initReq.Err:


Should assert on the expected error here.

carlaKC · 2022-01-18T07:05:45Z

@roeierez still interested in getting this in? I can pick up review once its rebased.

Bosch-0 · 2022-05-26T10:12:04Z

Would be great to see this in LND. LDK recently merged in a similar feature: lightningdevkit/rust-lightning#1163

by setting this value to zero or near zero (as I'm guessing you'd like to do), you introduce a costless attack for who ever you open a channel with.

@Roasbeef you could also look at this in reverse if the channel opener had big trust issues with their partner they could increase the reserve so more is at stake. This would make sense if the local capacities of both parties differs substantially.

As @roeierez mentioned, this is more of a business feature (particularly for LSPs) but the default can stay at 1% and changes be opt-in only.

lightninglabs-deploy · 2022-09-22T21:04:30Z

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

ellemouton · 2022-09-29T11:04:38Z

@roeierez, Im gonna close this PR has it has been replaced by this one. I have kept you as commit author where it applies. Thanks again for the og pr!

roeierez force-pushed the configure_chan_reserve branch from 2bfa980 to be28948 Compare February 27, 2019 16:33

Roasbeef added enhancement Improvements to existing features / behaviour rpc Related to the RPC interface funding Related to the opening of new channels with funding transactions on the blockchain cli Related to the command line interface P3 might get fixed, nice to have labels Mar 5, 2019

cfromknecht added this to the 0.8 milestone Jul 23, 2019

wpaulino added the v0.8.0 label Jul 30, 2019

roeierez force-pushed the configure_chan_reserve branch from be28948 to a108889 Compare August 1, 2019 09:01

roeierez requested review from Roasbeef, cfromknecht and halseth as code owners August 1, 2019 09:01

roeierez force-pushed the configure_chan_reserve branch from a108889 to 6a368e6 Compare August 1, 2019 09:15

Roasbeef removed their request for review August 7, 2019 02:28

cfromknecht reviewed Aug 7, 2019

View reviewed changes

roeierez force-pushed the configure_chan_reserve branch from 6a368e6 to 80e19f0 Compare August 8, 2019 08:33

wpaulino requested review from wpaulino and removed request for halseth August 12, 2019 23:54

wpaulino suggested changes Aug 13, 2019

View reviewed changes

roeierez force-pushed the configure_chan_reserve branch 2 times, most recently from 85e8436 to 61d109c Compare August 27, 2019 09:10

wpaulino added v0.12 and removed v0.10 labels Jul 14, 2020

Roasbeef mentioned this pull request Sep 15, 2020

Bug: balance reported by /channels disagrees with what LND checks in payinvoice #4585

Closed

Roasbeef added this to the 0.13.0 milestone Oct 1, 2020

Roasbeef added abandoned up for grabs PRs which have been abandoned by their original authors and can be taken up by someone else labels Mar 23, 2021

Roasbeef removed this from the 0.13.0 milestone Mar 23, 2021

roeierez added 2 commits April 20, 2021 16:26

proto: Add remote_chan_reserve_sat field to OpenChannelRequest

36be46a

This commit is the basis of enabling the intiator of a channel to determine the amount of satoshis he requires the remote peer to reserve as a direct payment in his side of the channel As stated in Bolt 2.

roeierez force-pushed the configure_chan_reserve branch from 61d109c to f1a7479 Compare April 20, 2021 14:57

roeierez added 2 commits April 20, 2021 20:20

lncli: Add lncli support to determine remote channel reservation.

d732e18

This commit adds the lncli support to configure the remote channel reservation value.

roeierez force-pushed the configure_chan_reserve branch 3 times, most recently from ea6e106 to cf4e796 Compare April 20, 2021 18:20

fundingmanager: add tests for remote chanel reserve

d9c3f5d

This commit adds three test cases for the remote channel reserve parameter: 1. custom valid value. 2. non valid value bellow dust. 3. non valid value over capacity.

roeierez force-pushed the configure_chan_reserve branch from cf4e796 to d9c3f5d Compare April 20, 2021 19:31

roeierez requested review from cfromknecht and wpaulino April 20, 2021 20:47

wpaulino reviewed May 3, 2021

View reviewed changes

carlaKC removed the request for review from cfromknecht January 18, 2022 07:05

Roasbeef linked an issue Aug 19, 2022 that may be closed by this pull request

startup option for channel_reserve_satoshis (LND) #1083

Closed

hsjoberg mentioned this pull request Aug 28, 2022

Remove channel reserve BlixtWallet/blixt-wallet#812

Open

ellemouton mentioned this pull request Sep 29, 2022

multi: configurable remote chan reserve #6956

Merged

ellemouton closed this Sep 29, 2022

Conversation

roeierez commented Feb 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roeierez commented Jul 12, 2019

Uh oh!

halseth commented Jul 18, 2019

Uh oh!

roeierez commented Jul 18, 2019

Uh oh!

wpaulino commented Jul 30, 2019

Uh oh!

roeierez commented Aug 1, 2019

Uh oh!

Roasbeef commented Aug 7, 2019

Uh oh!

roeierez commented Aug 7, 2019

Uh oh!

cfromknecht left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wpaulino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roeierez commented Aug 13, 2019

Uh oh!

roeierez commented Aug 27, 2019

Uh oh!

halseth commented Sep 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlaKC commented Jan 18, 2022

Uh oh!

Bosch-0 commented May 26, 2022

Uh oh!

lightninglabs-deploy commented Sep 22, 2022

Uh oh!

ellemouton commented Sep 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

roeierez commented Feb 26, 2019 •

edited

Loading