lnwallet: cleanup of reservation.CommitConstraints, and move RemoteChanConstraints.

This commit adds some more comments and checks to
reservation.CommitConstraints, including making
MinHTLC value one of the passed constraints.

RemoteChanConstraints is also moved out of
reservation.

lnwallet/interface_test.go

@@ -296,7 +296,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
 	}
 	aliceChanReservation.SetNumConfsRequired(numReqConfs)
 	aliceChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
-		lnwire.NewMSatFromSatoshis(fundingAmount), 10)
+		lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10)
 
 	// The channel reservation should now be populated with a multi-sig key
 	// from our HD chain, a change output with 3 BTC, and 2 outputs
@@ -319,7 +319,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("bob unable to init channel reservation: %v", err)
 	}
 	bobChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
-		lnwire.NewMSatFromSatoshis(fundingAmount), 10)
+		lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10)
 	bobChanReservation.SetNumConfsRequired(numReqConfs)
 
 	assertContributionInitPopulated(t, bobChanReservation.OurContribution())
@@ -649,7 +649,7 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 	}
 	aliceChanReservation.SetNumConfsRequired(numReqConfs)
 	aliceChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
-		lnwire.NewMSatFromSatoshis(fundingAmt), 10)
+		lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10)
 
 	// Verify all contribution fields have been set properly.
 	aliceContribution := aliceChanReservation.OurContribution()
@@ -661,7 +661,6 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("coin selection failed, should have one change outputs, "+
 			"instead have: %v", len(aliceContribution.ChangeOutputs))
 	}
-	aliceContribution.CsvDelay = csvDelay
 	assertContributionInitPopulated(t, aliceContribution)
 
 	// Next, Bob receives the initial request, generates a corresponding
@@ -673,12 +672,11 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("unable to create bob reservation: %v", err)
 	}
 	bobChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
-		lnwire.NewMSatFromSatoshis(fundingAmt), 10)
+		lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10)
 	bobChanReservation.SetNumConfsRequired(numReqConfs)
 
 	// We'll ensure that Bob's contribution also gets generated properly.
 	bobContribution := bobChanReservation.OurContribution()
-	bobContribution.CsvDelay = csvDelay
 	assertContributionInitPopulated(t, bobContribution)
 
 	// With his contribution generated, he can now process Alice's

lnwallet/reservation.go

@@ -277,92 +277,68 @@ func (r *ChannelReservation) RegisterMinHTLC(minHTLC lnwire.MilliSatoshi) {
 // will also attempt to verify the constraints for sanity, returning an error
 // if the parameters are seemed unsound.
 func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16,
-	maxValueInFlight lnwire.MilliSatoshi, chanReserve btcutil.Amount) error {
+	maxValueInFlight, minHtlc lnwire.MilliSatoshi,
+	chanReserve btcutil.Amount) error {
 
 	r.Lock()
 	defer r.Unlock()
 
-	// Fail if csvDelay is excessively large.
+	// Fail if we consider csvDelay excessively large.
+	// TODO(halseth): find a more scientific choice of value.
 	if csvDelay > 10000 {
 		return fmt.Errorf("csvDelay is too large: %d", csvDelay)
 	}
 
-	// Fail if the channel reserve is set to greater than 20% of the
+	// Fail if we consider the channel reserve to be too large.
+	// We currently fail if it is greater than 20% of the
 	// channel capacity.
-	maxChanReserve := (r.partialState.Capacity + 4) / 5
+	maxChanReserve := r.partialState.Capacity / 5
 	if chanReserve > maxChanReserve {
 		return fmt.Errorf("chanReserve is too large: %g",
 			chanReserve.ToBTC())
 	}
 
-	// Fail if the dust limit is lower than the DefaultDustLimit()
+	// Fail if the minimum HTLC value is too large. If this is
-	if r.ourContribution.DustLimit < DefaultDustLimit() {
+	// too large, the channel won't be useful for sending small
-		return fmt.Errorf("dust limit is too small: %g",
+	// payments. This limit is currently set to maxValueInFlight,
-			r.ourContribution.DustLimit.ToBTC())
+	// effictively letting the remote setting this as large as
+	// it wants.
+	// TODO(halseth): set a reasonable/dynamic value.
+	if minHtlc > maxValueInFlight {
+		return fmt.Errorf("minimum HTLC value is too large: %g",
+			r.ourContribution.MinHTLC.ToBTC())
 	}
 
 	// Fail if maxHtlcs is above the maximum allowed number of 483.
+	// This number is specified in BOLT-02.
 	if maxHtlcs > uint16(MaxHTLCNumber/2) {
 		return fmt.Errorf("maxHtlcs is too large: %d", maxHtlcs)
 	}
 
-	// Fail if maxHtlcs is too small.
+	// Fail if we consider maxHtlcs too small. If this is too small
-	if maxHtlcs < 5 {
+	// we cannot offer many HTLCs to the remote.
+	const minNumHtlc = 5
+	if maxHtlcs < minNumHtlc {
 		return fmt.Errorf("maxHtlcs is too small: %d", maxHtlcs)
 	}
 
-	// Fail if maxValueInFlight is too large.
+	// Fail if we consider maxValueInFlight too small. We currently
-	if maxValueInFlight > lnwire.NewMSatFromSatoshis(
+	// require the remote to at least allow minNumHtlc * minHtlc
-		r.partialState.Capacity-chanReserve) {
+	// in flight.
-		return fmt.Errorf("maxValueInFlight is too large: %g",
+	if maxValueInFlight < minNumHtlc*minHtlc {
-			maxValueInFlight.ToBTC())
-	}
-
-	// Fail if maxValueInFlight is too small.
-	if maxValueInFlight < r.ourContribution.MinHTLC {
 		return fmt.Errorf("maxValueInFlight is too small: %g",
 			maxValueInFlight.ToBTC())
 	}
 
-	// Fail if the minimum HTLC value is too large
-	if r.ourContribution.MinHTLC > maxValueInFlight {
-		return fmt.Errorf("minimum HTLC value is too large: %g",
-			r.ourContribution.MinHTLC.ToBTC())
-	}
-
 	r.ourContribution.ChannelConfig.CsvDelay = csvDelay
 	r.ourContribution.ChannelConfig.ChanReserve = chanReserve
 	r.ourContribution.ChannelConfig.MaxAcceptedHtlcs = maxHtlcs
 	r.ourContribution.ChannelConfig.MaxPendingAmount = maxValueInFlight
+	r.ourContribution.ChannelConfig.MinHTLC = minHtlc
 
 	return nil
 }
 
-// RemoteChanConstraints returns our desired parameters which constraint the
-// type of commitment transactions that the remote party can extend for our
-// current state. In order to ensure that we only accept sane states, we'll
-// specify: the required reserve the remote party must uphold, the max value in
-// flight, and the maximum number of HTLC's that can propose in a state.
-func (r *ChannelReservation) RemoteChanConstraints() (btcutil.Amount, lnwire.MilliSatoshi, uint16) {
-	chanCapacity := r.partialState.Capacity
-
-	// TODO(roasbeef): move csv delay calculation into func?
-
-	// By default, we'll require them to maintain at least 1% of the total
-	// channel capacity at all times. This is the absolute amount the
-	// settled balance of the remote party must be above at *all* times.
-	chanReserve := (chanCapacity) / 100
-
-	// We'll allow them to fully utilize the full bandwidth of the channel,
-	// minus our required reserve.
-	maxValue := lnwire.NewMSatFromSatoshis(chanCapacity - chanReserve)
-
-	// Finally, we'll permit them to utilize the full channel bandwidth
-	maxHTLCs := uint16(MaxHTLCNumber / 2)
-
-	return chanReserve, maxValue, maxHTLCs
-}
-
 // OurContribution returns the wallet's fully populated contribution to the
 // pending payment channel. See 'ChannelContribution' for further details
 // regarding the contents of a contribution.