funding+lnwallet: validate ChannelReserve is above DustLimit

This is necessary and is implied by BOLT#02. Both ChannelReserve
parameters should be above both DustLimit parameters. Otherwise,
it is possible for one side to have nothing at stake.

funding/manager.go

@@ -1340,7 +1340,7 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer,
 		CsvDelay:         msg.CsvDelay,
 	}
 	err = reservation.CommitConstraints(
-		channelConstraints, f.cfg.MaxLocalCSVDelay,
+		channelConstraints, f.cfg.MaxLocalCSVDelay, true,
 	)
 	if err != nil {
 		log.Errorf("Unacceptable channel constraints: %v", err)
@@ -1386,7 +1386,19 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer,
 		remoteCsvDelay = acceptorResp.CSVDelay
 	}
 
-	chanReserve := f.cfg.RequiredRemoteChanReserve(amt, msg.DustLimit)
+	// If our default dust limit was above their ChannelReserve, we change
+	// it to the ChannelReserve. We must make sure the ChannelReserve we
+	// send in the AcceptChannel message is above both dust limits.
+	// Therefore, take the maximum of msg.DustLimit and our dust limit.
+	//
+	// NOTE: Even with this bounding, the ChannelAcceptor may return an
+	// BOLT#02-invalid ChannelReserve.
+	maxDustLimit := reservation.OurContribution().DustLimit
+	if msg.DustLimit > maxDustLimit {
+		maxDustLimit = msg.DustLimit
+	}
+
+	chanReserve := f.cfg.RequiredRemoteChanReserve(amt, maxDustLimit)
 	if acceptorResp.Reserve != 0 {
 		chanReserve = acceptorResp.Reserve
 	}
@@ -1576,7 +1588,7 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 		CsvDelay:         msg.CsvDelay,
 	}
 	err = resCtx.reservation.CommitConstraints(
-		channelConstraints, resCtx.maxLocalCsv,
+		channelConstraints, resCtx.maxLocalCsv, false,
 	)
 	if err != nil {
 		log.Warnf("Unacceptable channel constraints: %v", err)
@@ -1586,8 +1598,11 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 
 	// As they've accepted our channel constraints, we'll regenerate them
 	// here so we can properly commit their accepted constraints to the
-	// reservation.
+	// reservation. Also make sure that we re-generate the ChannelReserve
-	chanReserve := f.cfg.RequiredRemoteChanReserve(resCtx.chanAmt, msg.DustLimit)
+	// 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

lnwallet/errors.go

@@ -152,6 +152,14 @@ func ErrChanTooLarge(chanSize, maxChanSize btcutil.Amount) ReservationError {
 	}
 }
 
+// ErrInvalidDustLimit returns an error indicating that a proposed DustLimit
+// was rejected.
+func ErrInvalidDustLimit(dustLimit btcutil.Amount) ReservationError {
+	return ReservationError{
+		fmt.Errorf("dust limit %v is invalid", dustLimit),
+	}
+}
+
 // ErrHtlcIndexAlreadyFailed is returned when the HTLC index has already been
 // failed, but has not been committed by our commitment state.
 type ErrHtlcIndexAlreadyFailed uint64

lnwallet/reservation.go

@@ -394,7 +394,7 @@ func (r *ChannelReservation) SetNumConfsRequired(numConfs uint16) {
 // 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,
-	maxLocalCSVDelay uint16) error {
+	maxLocalCSVDelay uint16, responder bool) error {
 	r.Lock()
 	defer r.Unlock()
 
@@ -409,6 +409,13 @@ func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints,
 		return ErrChanReserveTooSmall(c.ChanReserve, c.DustLimit)
 	}
 
+	// Validate against the maximum-sized witness script dust limit, and
+	// also ensure that the DustLimit is not too large.
+	maxWitnessLimit := DustLimitForSize(input.UnknownWitnessSize)
+	if c.DustLimit < maxWitnessLimit || c.DustLimit > 3*maxWitnessLimit {
+		return ErrInvalidDustLimit(c.DustLimit)
+	}
+
 	// 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 / 5
@@ -449,7 +456,7 @@ func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints,
 
 	// Our dust limit should always be less than or equal to our proposed
 	// channel reserve.
-	if r.ourContribution.DustLimit > c.ChanReserve {
+	if responder && r.ourContribution.DustLimit > c.ChanReserve {
 		r.ourContribution.DustLimit = c.ChanReserve
 	}
 
@@ -462,6 +469,31 @@ func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints,
 	return nil
 }
 
+// validateReserveBounds checks that both ChannelReserve values are above both
+// DustLimit values. This not only avoids stuck channels, but is also mandated
+// by BOLT#02 even if it's not explicit. This returns true if the bounds are
+// valid. This function should be called with the lock held.
+func (r *ChannelReservation) validateReserveBounds() bool {
+	ourDustLimit := r.ourContribution.DustLimit
+	ourRequiredReserve := r.ourContribution.ChanReserve
+	theirDustLimit := r.theirContribution.DustLimit
+	theirRequiredReserve := r.theirContribution.ChanReserve
+
+	// We take the smaller of the two ChannelReserves and compare it
+	// against the larger of the two DustLimits.
+	minChanReserve := ourRequiredReserve
+	if minChanReserve > theirRequiredReserve {
+		minChanReserve = theirRequiredReserve
+	}
+
+	maxDustLimit := ourDustLimit
+	if maxDustLimit < theirDustLimit {
+		maxDustLimit = theirDustLimit
+	}
+
+	return minChanReserve >= maxDustLimit
+}
+
 // OurContribution returns the wallet's fully populated contribution to the
 // pending payment channel. See 'ChannelContribution' for further details
 // regarding the contents of a contribution.

lnwallet/test/test_interface.go

@@ -454,7 +454,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
 		CsvDelay:         csvDelay,
 	}
 	err = aliceChanReservation.CommitConstraints(
-		channelConstraints, defaultMaxLocalCsvDelay,
+		channelConstraints, defaultMaxLocalCsvDelay, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to verify constraints: %v", err)
@@ -490,7 +490,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("bob unable to init channel reservation: %v", err)
 	}
 	err = bobChanReservation.CommitConstraints(
-		channelConstraints, defaultMaxLocalCsvDelay,
+		channelConstraints, defaultMaxLocalCsvDelay, true,
 	)
 	if err != nil {
 		t.Fatalf("unable to verify constraints: %v", err)
@@ -904,7 +904,7 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 		CsvDelay:         csvDelay,
 	}
 	err = aliceChanReservation.CommitConstraints(
-		channelConstraints, defaultMaxLocalCsvDelay,
+		channelConstraints, defaultMaxLocalCsvDelay, false,
 	)
 	if err != nil {
 		t.Fatalf("unable to verify constraints: %v", err)
@@ -947,7 +947,7 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("unable to create bob reservation: %v", err)
 	}
 	err = bobChanReservation.CommitConstraints(
-		channelConstraints, defaultMaxLocalCsvDelay,
+		channelConstraints, defaultMaxLocalCsvDelay, true,
 	)
 	if err != nil {
 		t.Fatalf("unable to verify constraints: %v", err)

lnwallet/wallet.go

@@ -1266,6 +1266,13 @@ func (l *LightningWallet) handleContributionMsg(req *addContributionMsg) {
 	theirContribution := req.contribution
 	ourContribution := pendingReservation.ourContribution
 
+	// Perform bounds-checking on both ChannelReserve and DustLimit
+	// parameters.
+	if !pendingReservation.validateReserveBounds() {
+		req.err <- fmt.Errorf("invalid reserve and dust bounds")
+		return
+	}
+
 	var (
 		chanPoint *wire.OutPoint
 		err       error
@@ -1564,9 +1571,6 @@ func (l *LightningWallet) handleSingleContribution(req *addSingleContributionMsg
 	pendingReservation.Lock()
 	defer pendingReservation.Unlock()
 
-	// TODO(roasbeef): verify sanity of remote party's parameters, fail if
-	// disagree
-
 	// Validate that the remote's UpfrontShutdownScript is a valid script
 	// if it's set.
 	shutdown := req.contribution.UpfrontShutdown
@@ -1584,6 +1588,14 @@ func (l *LightningWallet) handleSingleContribution(req *addSingleContributionMsg
 	theirContribution := pendingReservation.theirContribution
 	chanState := pendingReservation.partialState
 
+	// Perform bounds checking on both ChannelReserve and DustLimit
+	// parameters. The ChannelReserve may have been changed by the
+	// ChannelAcceptor RPC, so this is necessary.
+	if !pendingReservation.validateReserveBounds() {
+		req.err <- fmt.Errorf("invalid reserve and dust bounds")
+		return
+	}
+
 	// Initialize an empty sha-chain for them, tracking the current pending
 	// revocation hash (we don't yet know the preimage so we can't add it
 	// to the chain).