From b220c47ce74fe7a47d436a80a31309a6f43a3c9b Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 29 Jan 2019 17:56:28 -0800 Subject: [PATCH 1/2] Revert "funding+lnwallet: ensure max_htlc_value_in_flight smaller than capacity" This reverts commit 4aa52d267f000f84caf912c62fc14a5b8e7cacb5. It turns out that the other implementations set values for this field which aren't based on the actual capacity of the channel. As a result, we'll no reject most of their channel offerings, since they may offer a value of a max `uint64` or something else hard coded that's above the size of the channel. As a result, we're reverting this check for now to maintain proper compatibility. --- fundingmanager.go | 6 ++---- lnwallet/errors.go | 10 ---------- lnwallet/interface_test.go | 16 ++++------------ lnwallet/reservation.go | 14 ++------------ 4 files changed, 8 insertions(+), 38 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 772cc319..b5127c0f 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -1090,7 +1090,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { MaxAcceptedHtlcs: msg.MaxAcceptedHTLCs, CsvDelay: msg.CsvDelay, } - err = reservation.CommitConstraints(channelConstraints, amt) + err = reservation.CommitConstraints(channelConstraints) if err != nil { fndgLog.Errorf("Unacceptable channel constraints: %v", err) f.failFundingFlow(fmsg.peer, fmsg.msg.PendingChannelID, err) @@ -1254,9 +1254,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { MaxAcceptedHtlcs: msg.MaxAcceptedHTLCs, CsvDelay: msg.CsvDelay, } - err = resCtx.reservation.CommitConstraints( - channelConstraints, resCtx.chanAmt, - ) + err = resCtx.reservation.CommitConstraints(channelConstraints) if err != nil { fndgLog.Warnf("Unacceptable channel constraints: %v", err) f.failFundingFlow(fmsg.peer, fmsg.msg.PendingChannelID, err) diff --git a/lnwallet/errors.go b/lnwallet/errors.go index d72caf65..79ab10f7 100644 --- a/lnwallet/errors.go +++ b/lnwallet/errors.go @@ -132,16 +132,6 @@ func ErrNumConfsTooLarge(numConfs, maxNumConfs uint32) error { } } -// ErrMaxValueInFlightTooLarge returns an error indicating that the 'max HTLC -// value in flight' the remote required is too large to be accepted. -func ErrMaxValueInFlightTooLarge(maxValInFlight, - maxMaxValInFlight lnwire.MilliSatoshi) ReservationError { - return ReservationError{ - fmt.Errorf("maxValueInFlight too large: %v, max is %v", - maxValInFlight, maxMaxValInFlight), - } -} - // ErrChanTooSmall returns an error indicating that an incoming channel request // was too small. We'll reject any incoming channels if they're below our // configured value for the min channel size we'll accept. diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 7ed0dfe9..4549a266 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -437,9 +437,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, CsvDelay: csvDelay, } - err = aliceChanReservation.CommitConstraints( - channelConstraints, fundingAmount*2, - ) + err = aliceChanReservation.CommitConstraints(channelConstraints) if err != nil { t.Fatalf("unable to verify constraints: %v", err) } @@ -473,9 +471,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, if err != nil { t.Fatalf("bob unable to init channel reservation: %v", err) } - err = bobChanReservation.CommitConstraints( - channelConstraints, fundingAmount*2, - ) + err = bobChanReservation.CommitConstraints(channelConstraints) if err != nil { t.Fatalf("unable to verify constraints: %v", err) } @@ -873,9 +869,7 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, CsvDelay: csvDelay, } - err = aliceChanReservation.CommitConstraints( - channelConstraints, fundingAmt, - ) + err = aliceChanReservation.CommitConstraints(channelConstraints) if err != nil { t.Fatalf("unable to verify constraints: %v", err) } @@ -909,9 +903,7 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, if err != nil { t.Fatalf("unable to create bob reservation: %v", err) } - err = bobChanReservation.CommitConstraints( - channelConstraints, fundingAmt, - ) + err = bobChanReservation.CommitConstraints(channelConstraints) if err != nil { t.Fatalf("unable to verify constraints: %v", err) } diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index 1a684f25..a32acfa2 100644 --- a/lnwallet/reservation.go +++ b/lnwallet/reservation.go @@ -286,9 +286,7 @@ func (r *ChannelReservation) SetNumConfsRequired(numConfs uint16) { // of satoshis that can be transferred in a single commitment. This function // will also attempt to verify the constraints for sanity, returning an error // if the parameters are seemed unsound. -func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints, - capacity btcutil.Amount) error { - +func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints) error { r.Lock() defer r.Unlock() @@ -343,15 +341,7 @@ func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints, ) } - // Fail if the maxValueInFlight is greater than the channel capacity. - capacityMsat := lnwire.NewMSatFromSatoshis(capacity) - if c.MaxPendingAmount > capacityMsat { - return ErrMaxValueInFlightTooLarge( - c.MaxPendingAmount, capacityMsat, - ) - } - - // Our dust limit should always be less than or equal our proposed + // Our dust limit should always be less than or equal to our proposed // channel reserve. if r.ourContribution.DustLimit > c.ChanReserve { r.ourContribution.DustLimit = c.ChanReserve From b4c5833325f8a5e12f0c1338cbc206b1366917ac Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 29 Jan 2019 18:30:57 -0800 Subject: [PATCH 2/2] Revert "fundingmanager_test: add TestFundingManagerRejectInvalidMaxValueInFlight" This reverts commit eb618051208dbd8bb0c160001052b5cc36174888. --- fundingmanager_test.go | 66 ------------------------------------------ 1 file changed, 66 deletions(-) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index 5802083c..196712df 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -2657,69 +2657,3 @@ func TestFundingManagerMaxConfs(t *testing.T) { string(err.Data)) } } - -// TestFundingManagerRejectInvalidMaxValueInFlight makes sure that the funding -// manager will act accordingly when the remote is requiring us to use a -// max_value_in_flight larger than the channel capacity. -func TestFundingManagerRejectInvalidMaxValueInFlight(t *testing.T) { - alice, bob := setupFundingManagers(t, defaultMaxPendingChannels) - defer tearDownFundingManagers(t, alice, bob) - - localAmt := btcutil.Amount(500000) - pushAmt := btcutil.Amount(0) - capacity := localAmt + pushAmt - - // Make Alice require a max_htlc_value_in_flight greater than the - // channel capacity. - alice.fundingMgr.cfg.RequiredRemoteMaxValue = func( - _ btcutil.Amount) lnwire.MilliSatoshi { - return lnwire.NewMSatFromSatoshis(capacity) + 100 - } - - // Create a funding request and start the workflow. - updateChan := make(chan *lnrpc.OpenStatusUpdate) - errChan := make(chan error, 1) - initReq := &openChanReq{ - targetPubkey: bob.privKey.PubKey(), - chainHash: *activeNetParams.GenesisHash, - localFundingAmt: 500000, - pushAmt: lnwire.NewMSatFromSatoshis(10), - private: true, - updates: updateChan, - err: errChan, - } - - alice.fundingMgr.initFundingWorkflow(bob, initReq) - - // Alice should have sent the OpenChannel message to Bob. - var aliceMsg lnwire.Message - select { - case aliceMsg = <-alice.msgChan: - case err := <-initReq.err: - t.Fatalf("error init funding workflow: %v", err) - case <-time.After(time.Second * 5): - t.Fatalf("alice did not send OpenChannel message") - } - - openChannelReq, ok := aliceMsg.(*lnwire.OpenChannel) - if !ok { - errorMsg, gotError := aliceMsg.(*lnwire.Error) - if gotError { - t.Fatalf("expected OpenChannel to be sent "+ - "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) - } - t.Fatalf("expected OpenChannel to be sent from "+ - "alice, instead got %T", aliceMsg) - } - - // Let Bob handle the init message. - bob.fundingMgr.processFundingOpen(openChannelReq, alice) - - // Assert Bob responded with an ErrMaxValueInFlightTooLarge error. - err := assertFundingMsgSent(t, bob.msgChan, "Error").(*lnwire.Error) - if !strings.Contains(string(err.Data), "maxValueInFlight too large") { - t.Fatalf("expected ErrMaxValueInFlightTooLarge error, "+ - "got \"%v\"", string(err.Data)) - } -}