diff --git a/funding/manager.go b/funding/manager.go index 43f4a8991..dc23128f1 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -142,10 +142,11 @@ type reservationWithCtx struct { forwardingPolicy htlcswitch.ForwardingPolicy // Constraints we require for the remote. - remoteCsvDelay uint16 - remoteMinHtlc lnwire.MilliSatoshi - remoteMaxValue lnwire.MilliSatoshi - remoteMaxHtlcs uint16 + remoteCsvDelay uint16 + remoteMinHtlc lnwire.MilliSatoshi + remoteMaxValue lnwire.MilliSatoshi + remoteMaxHtlcs uint16 + remoteChanReserve btcutil.Amount // maxLocalCsv is the maximum csv we will accept from the remote. maxLocalCsv uint16 @@ -224,6 +225,10 @@ type InitFundingMsg struct { // RemoteCsvDelay is the CSV delay we require for the remote peer. RemoteCsvDelay uint16 + // RemoteChanReserve is the channel reserve we required for the remote + // peer. + RemoteChanReserve btcutil.Amount + // MinConfs indicates the minimum number of confirmations that each // output selected to fund the channel should satisfy. MinConfs int32 @@ -1620,17 +1625,18 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, f.activeReservations[peerIDKey] = make(pendingChannels) } resCtx := &reservationWithCtx{ - reservation: reservation, - chanAmt: amt, - forwardingPolicy: forwardingPolicy, - remoteCsvDelay: remoteCsvDelay, - remoteMinHtlc: minHtlc, - remoteMaxValue: remoteMaxValue, - remoteMaxHtlcs: maxHtlcs, - maxLocalCsv: f.cfg.MaxLocalCSVDelay, - channelType: msg.ChannelType, - err: make(chan error, 1), - peer: peer, + reservation: reservation, + chanAmt: amt, + forwardingPolicy: forwardingPolicy, + remoteCsvDelay: remoteCsvDelay, + remoteMinHtlc: minHtlc, + remoteMaxValue: remoteMaxValue, + remoteMaxHtlcs: maxHtlcs, + remoteChanReserve: chanReserve, + maxLocalCsv: f.cfg.MaxLocalCSVDelay, + channelType: msg.ChannelType, + err: make(chan error, 1), + peer: peer, } f.activeReservations[peerIDKey][msg.PendingChannelID] = resCtx f.resMtx.Unlock() @@ -1850,14 +1856,6 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer, return } - // As they've accepted our channel constraints, we'll regenerate them - // here so we can properly commit their accepted constraints to the - // reservation. Also make sure that we re-generate the ChannelReserve - // with our dust limit or we can get stuck channels. - chanReserve := f.cfg.RequiredRemoteChanReserve( - resCtx.chanAmt, resCtx.reservation.OurContribution().DustLimit, - ) - // The remote node has responded with their portion of the channel // contribution. At this point, we can process their contribution which // allows us to construct and sign both the commitment transaction, and @@ -1868,7 +1866,7 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer, ChannelConstraints: channeldb.ChannelConstraints{ DustLimit: msg.DustLimit, MaxPendingAmount: resCtx.remoteMaxValue, - ChanReserve: chanReserve, + ChanReserve: resCtx.remoteChanReserve, MinHTLC: resCtx.remoteMinHtlc, MaxAcceptedHtlcs: resCtx.remoteMaxHtlcs, CsvDelay: resCtx.remoteCsvDelay, @@ -3888,6 +3886,7 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { maxValue = msg.MaxValueInFlight maxHtlcs = msg.MaxHtlcs maxCSV = msg.MaxLocalCsv + chanReserve = msg.RemoteChanReserve ) // If no maximum CSV delay was set for this channel, we use our default @@ -4092,35 +4091,6 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { forwardingPolicy.FeeRate = lnwire.MilliSatoshi(*feeRate) } - // If a pending channel map for this peer isn't already created, then - // we create one, ultimately allowing us to track this pending - // reservation within the target peer. - peerIDKey := newSerializedKey(peerKey) - f.resMtx.Lock() - if _, ok := f.activeReservations[peerIDKey]; !ok { - f.activeReservations[peerIDKey] = make(pendingChannels) - } - - resCtx := &reservationWithCtx{ - chanAmt: capacity, - forwardingPolicy: forwardingPolicy, - remoteCsvDelay: remoteCsvDelay, - remoteMinHtlc: minHtlcIn, - remoteMaxValue: maxValue, - remoteMaxHtlcs: maxHtlcs, - maxLocalCsv: maxCSV, - channelType: msg.ChannelType, - reservation: reservation, - peer: msg.Peer, - updates: msg.Updates, - err: msg.Err, - } - f.activeReservations[peerIDKey][chanID] = resCtx - f.resMtx.Unlock() - - // Update the timestamp once the InitFundingMsg has been handled. - defer resCtx.updateTimestamp() - // Once the reservation has been created, and indexed, queue a funding // request to the remote peer, kicking off the funding workflow. ourContribution := reservation.OurContribution() @@ -4131,10 +4101,43 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) { log.Infof("Dust limit for pendingID(%x): %v", chanID, ourDustLimit) - // Finally, we'll use the current value of the channels and our default - // policy to determine of required commitment constraints for the - // remote party. - chanReserve := f.cfg.RequiredRemoteChanReserve(capacity, ourDustLimit) + // If the channel reserve is not specified, then we calculate an + // appropriate amount here. + if chanReserve == 0 { + chanReserve = f.cfg.RequiredRemoteChanReserve( + capacity, ourDustLimit, + ) + } + + // If a pending channel map for this peer isn't already created, then + // we create one, ultimately allowing us to track this pending + // reservation within the target peer. + peerIDKey := newSerializedKey(peerKey) + f.resMtx.Lock() + if _, ok := f.activeReservations[peerIDKey]; !ok { + f.activeReservations[peerIDKey] = make(pendingChannels) + } + + resCtx := &reservationWithCtx{ + chanAmt: capacity, + forwardingPolicy: forwardingPolicy, + remoteCsvDelay: remoteCsvDelay, + remoteMinHtlc: minHtlcIn, + remoteMaxValue: maxValue, + remoteMaxHtlcs: maxHtlcs, + remoteChanReserve: chanReserve, + maxLocalCsv: maxCSV, + channelType: msg.ChannelType, + reservation: reservation, + peer: msg.Peer, + updates: msg.Updates, + err: msg.Err, + } + f.activeReservations[peerIDKey][chanID] = resCtx + f.resMtx.Unlock() + + // Update the timestamp once the InitFundingMsg has been handled. + defer resCtx.updateTimestamp() // Check the sanity of the selected channel constraints. channelConstraints := &channeldb.ChannelConstraints{ diff --git a/funding/manager_test.go b/funding/manager_test.go index 849005386..76d6ad046 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -2765,6 +2765,7 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { const minHtlcIn = 1234 const maxValueInFlight = 50000 const fundingAmt = 5000000 + const chanReserve = 100000 // Use custom channel fees. // These will show up in the channel reservation context @@ -2785,19 +2786,20 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // workflow. errChan := make(chan error, 1) initReq := &InitFundingMsg{ - Peer: bob, - TargetPubkey: bob.privKey.PubKey(), - ChainHash: *fundingNetParams.GenesisHash, - LocalFundingAmt: localAmt, - PushAmt: lnwire.NewMSatFromSatoshis(pushAmt), - Private: false, - MaxValueInFlight: maxValueInFlight, - MinHtlcIn: minHtlcIn, - RemoteCsvDelay: csvDelay, - Updates: updateChan, - Err: errChan, - BaseFee: &baseFee, - FeeRate: &feeRate, + Peer: bob, + TargetPubkey: bob.privKey.PubKey(), + ChainHash: *fundingNetParams.GenesisHash, + LocalFundingAmt: localAmt, + PushAmt: lnwire.NewMSatFromSatoshis(pushAmt), + Private: false, + MaxValueInFlight: maxValueInFlight, + MinHtlcIn: minHtlcIn, + RemoteCsvDelay: csvDelay, + RemoteChanReserve: chanReserve, + Updates: updateChan, + Err: errChan, + BaseFee: &baseFee, + FeeRate: &feeRate, } alice.fundingMgr.InitFundingWorkflow(initReq) @@ -2842,6 +2844,12 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { maxValueInFlight, openChannelReq.MaxValueInFlight) } + // Check that the custom remoteChanReserve value is sent. + if openChannelReq.ChannelReserve != chanReserve { + t.Fatalf("expected OpenChannel to have chanReserve %v, got %v", + chanReserve, openChannelReq.ChannelReserve) + } + chanID := openChannelReq.PendingChannelID // Let Bob handle the init message. @@ -3133,6 +3141,88 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { } } +// TestFundingManagerInvalidChanReserve ensures proper validation is done on +// remoteChanReserve parameter sent to open channel. +func TestFundingManagerInvalidChanReserve(t *testing.T) { + t.Parallel() + + var ( + fundingAmt = btcutil.Amount(500000) + pushAmt = lnwire.NewMSatFromSatoshis(10) + genesisHash = *fundingNetParams.GenesisHash + ) + + tests := []struct { + name string + chanReserve btcutil.Amount + expectErr bool + errorContains string + }{ + { + name: "Use default chan reserve", + chanReserve: 0, + }, + { + name: "Above dust but below 1% of the capacity", + chanReserve: 400, + }, + { + name: "Channel reserve below dust", + chanReserve: 300, + expectErr: true, + errorContains: "channel reserve of 300 sat is too " + + "small", + }, + { + name: "Channel reserve more than 20% of the " + + "channel capacity", + chanReserve: fundingAmt, + expectErr: true, + errorContains: "channel reserve is too large", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + alice, bob := setupFundingManagers(t) + defer tearDownFundingManagers(t, alice, bob) + + // Create a funding request and start the workflow. + updateChan := make(chan *lnrpc.OpenStatusUpdate) + errChan := make(chan error, 1) + initReq := &InitFundingMsg{ + Peer: bob, + TargetPubkey: bob.privKey.PubKey(), + ChainHash: genesisHash, + LocalFundingAmt: fundingAmt, + PushAmt: pushAmt, + Updates: updateChan, + RemoteChanReserve: test.chanReserve, + Err: errChan, + } + + alice.fundingMgr.InitFundingWorkflow(initReq) + + var err error + select { + case <-alice.msgChan: + case err = <-initReq.Err: + case <-time.After(time.Second * 5): + t.Fatalf("no message or error received") + } + + if !test.expectErr { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, test.errorContains) + }) + } +} + // TestFundingManagerMaxPendingChannels checks that trying to open another // channel with the same peer when MaxPending channels are pending fails. func TestFundingManagerMaxPendingChannels(t *testing.T) {