Merge pull request #1215 from wpaulino/fee-dust-adherence

lnwallet: enforce fee floor and dust-reserve adherence

fundingmanager.go

@@ -279,10 +279,10 @@ type fundingConfig struct {
 	RequiredRemoteDelay func(btcutil.Amount) uint16
 
 	// RequiredRemoteChanReserve is a function closure that, given the
-	// channel capacity, will return an appropriate amount for the remote
+	// channel capacity and dust limit, will return an appropriate amount
-	// peer's required channel reserve that is to be adhered to at all
+	// for the remote peer's required channel reserve that is to be adhered
-	// times.
+	// to at all times.
-	RequiredRemoteChanReserve func(btcutil.Amount) btcutil.Amount
+	RequiredRemoteChanReserve func(capacity, dustLimit btcutil.Amount) btcutil.Amount
 
 	// RequiredRemoteMaxValue is a function closure that, given the channel
 	// capacity, returns the amount of MilliSatoshis that our remote peer
@@ -994,10 +994,10 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) {
 	// party is attempting to dictate for our commitment transaction.
 	err = reservation.CommitConstraints(
 		msg.CsvDelay, msg.MaxAcceptedHTLCs, msg.MaxValueInFlight,
-		msg.HtlcMinimum, msg.ChannelReserve,
+		msg.HtlcMinimum, msg.ChannelReserve, msg.DustLimit,
 	)
 	if err != nil {
-		fndgLog.Errorf("Unaccaptable channel constraints: %v", err)
+		fndgLog.Errorf("Unacceptable channel constraints: %v", err)
 		f.failFundingFlow(fmsg.peerAddress.IdentityKey,
 			fmsg.msg.PendingChannelID, err,
 		)
@@ -1008,12 +1008,9 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) {
 		"amt=%v, push_amt=%v", numConfsReq, fmsg.msg.PendingChannelID,
 		amt, msg.PushAmount)
 
-	// Using the RequiredRemoteDelay closure, we'll compute the remote CSV
+	// Generate our required constraints for the remote party.
-	// delay we require given the total amount of funds within the channel.
 	remoteCsvDelay := f.cfg.RequiredRemoteDelay(amt)
-
+	chanReserve := f.cfg.RequiredRemoteChanReserve(amt, msg.DustLimit)
-	// We'll also generate our required constraints for the remote party,
-	chanReserve := f.cfg.RequiredRemoteChanReserve(amt)
 	maxValue := f.cfg.RequiredRemoteMaxValue(amt)
 	maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(amt)
 	minHtlc := f.cfg.DefaultRoutingPolicy.MinHTLC
@@ -1149,7 +1146,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) {
 	resCtx.reservation.SetNumConfsRequired(uint16(msg.MinAcceptDepth))
 	err = resCtx.reservation.CommitConstraints(
 		msg.CsvDelay, msg.MaxAcceptedHTLCs, msg.MaxValueInFlight,
-		msg.HtlcMinimum, msg.ChannelReserve,
+		msg.HtlcMinimum, msg.ChannelReserve, msg.DustLimit,
 	)
 	if err != nil {
 		fndgLog.Warnf("Unacceptable channel constraints: %v", err)
@@ -1161,7 +1158,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) {
 	// As they've accepted our channel constraints, we'll regenerate them
 	// here so we can properly commit their accepted constraints to the
 	// reservation.
-	chanReserve := f.cfg.RequiredRemoteChanReserve(resCtx.chanAmt)
+	chanReserve := f.cfg.RequiredRemoteChanReserve(resCtx.chanAmt, msg.DustLimit)
 	maxValue := f.cfg.RequiredRemoteMaxValue(resCtx.chanAmt)
 	maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(resCtx.chanAmt)
 
@@ -2573,7 +2570,7 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
 	// 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)
+	chanReserve := f.cfg.RequiredRemoteChanReserve(capacity, ourDustLimit)
 	maxValue := f.cfg.RequiredRemoteMaxValue(capacity)
 	maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(capacity)
 

fundingmanager_test.go

@@ -294,8 +294,15 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey,
 		RequiredRemoteDelay: func(amt btcutil.Amount) uint16 {
 			return 4
 		},
-		RequiredRemoteChanReserve: func(chanAmt btcutil.Amount) btcutil.Amount {
+		RequiredRemoteChanReserve: func(chanAmt,
-			return chanAmt / 100
+			dustLimit btcutil.Amount) btcutil.Amount {
+
+			reserve := chanAmt / 100
+			if reserve < dustLimit {
+				reserve = dustLimit
+			}
+
+			return reserve
 		},
 		RequiredRemoteMaxValue: func(chanAmt btcutil.Amount) lnwire.MilliSatoshi {
 			reserve := lnwire.NewMSatFromSatoshis(chanAmt / 100)

lnd.go

@@ -464,11 +464,20 @@ func lndMain() error {
 			cid := lnwire.NewChanIDFromOutPoint(&chanPoint)
 			return server.htlcSwitch.UpdateShortChanID(cid)
 		},
-		RequiredRemoteChanReserve: func(chanAmt btcutil.Amount) btcutil.Amount {
+		RequiredRemoteChanReserve: func(chanAmt,
+			dustLimit btcutil.Amount) btcutil.Amount {
+
 			// By default, we'll require the remote peer to maintain
 			// at least 1% of the total channel capacity at all
-			// times.
+			// times. If this value ends up dipping below the dust
-			return chanAmt / 100
+			// limit, then we'll use the dust limit itself as the
+			// reserve as required by BOLT #2.
+			reserve := chanAmt / 100
+			if reserve < dustLimit {
+				reserve = dustLimit
+			}
+
+			return reserve
 		},
 		RequiredRemoteMaxValue: func(chanAmt btcutil.Amount) lnwire.MilliSatoshi {
 			// By default, we'll allow the remote peer to fully

lnwallet/errors.go

@@ -59,6 +59,15 @@ func ErrCsvDelayTooLarge(remoteDelay, maxDelay uint16) ReservationError {
 	}
 }
 
+// ErrChanReserveTooSmall returns an error indicating that the channel reserve
+// the remote is requiring is too small to be accepted.
+func ErrChanReserveTooSmall(reserve, dustLimit btcutil.Amount) ReservationError {
+	return ReservationError{
+		fmt.Errorf("channel reserve of %v sat is too small, min is %v "+
+			"sat", int64(reserve), int64(dustLimit)),
+	}
+}
+
 // ErrChanReserveTooLarge returns an error indicating that the chan reserve the
 // remote is requiring, is too large to be accepted.
 func ErrChanReserveTooLarge(reserve,

lnwallet/fee_estimator.go

@@ -95,6 +95,12 @@ type BtcdFeeEstimator struct {
 	// actually produce fee estimates.
 	fallBackFeeRate SatPerVByte
 
+	// minFeeRate is the minimum relay fee, in sat/vbyte, of the backend
+	// node. This will be used as the default fee rate of a transaction when
+	// the estimated fee rate is too low to allow the transaction to
+	// propagate through the network.
+	minFeeRate SatPerVByte
+
 	btcdConn *rpcclient.Client
 }
 
@@ -128,6 +134,22 @@ func (b *BtcdFeeEstimator) Start() error {
 		return err
 	}
 
+	// Once the connection to the backend node has been established, we'll
+	// query it for its minimum relay fee.
+	info, err := b.btcdConn.GetInfo()
+	if err != nil {
+		return err
+	}
+
+	relayFee, err := btcutil.NewAmount(info.RelayFee)
+	if err != nil {
+		return err
+	}
+
+	// The fee rate is expressed in sat/KB, so we'll manually convert it to
+	// our desired sat/vbyte rate.
+	b.minFeeRate = SatPerVByte(relayFee / 1000)
+
 	return nil
 }
 
@@ -183,12 +205,20 @@ func (b *BtcdFeeEstimator) fetchEstimatePerVSize(
 	// The value returned is expressed in fees per KB, while we want
 	// fee-per-byte, so we'll divide by 1000 to map to satoshis-per-byte
 	// before returning the estimate.
-	satPerByte := satPerKB / 1000
+	satPerByte := SatPerVByte(satPerKB / 1000)
+
+	// Before proceeding, we'll make sure that this fee rate respects the
+	// minimum relay fee set on the backend node.
+	if satPerByte < b.minFeeRate {
+		walletLog.Debugf("Using backend node's minimum relay fee rate "+
+			"of %v sat/vbyte", b.minFeeRate)
+		satPerByte = b.minFeeRate
+	}
 
 	walletLog.Debugf("Returning %v sat/vbyte for conf target of %v",
 		int64(satPerByte), confTarget)
 
-	return SatPerVByte(satPerByte), nil
+	return satPerByte, nil
 }
 
 // A compile-time assertion to ensure that BtcdFeeEstimator implements the
@@ -204,6 +234,12 @@ type BitcoindFeeEstimator struct {
 	// actually produce fee estimates.
 	fallBackFeeRate SatPerVByte
 
+	// minFeeRate is the minimum relay fee, in sat/vbyte, of the backend
+	// node. This will be used as the default fee rate of a transaction when
+	// the estimated fee rate is too low to allow the transaction to
+	// propagate through the network.
+	minFeeRate SatPerVByte
+
 	bitcoindConn *rpcclient.Client
 }
 
@@ -235,6 +271,32 @@ func NewBitcoindFeeEstimator(rpcConfig rpcclient.ConnConfig,
 //
 // NOTE: This method is part of the FeeEstimator interface.
 func (b *BitcoindFeeEstimator) Start() error {
+	// Once the connection to the backend node has been established, we'll
+	// query it for its minimum relay fee. Since the `getinfo` RPC has been
+	// deprecated for `bitcoind`, we'll need to send a `getnetworkinfo`
+	// command as a raw request.
+	resp, err := b.bitcoindConn.RawRequest("getnetworkinfo", nil)
+	if err != nil {
+		return err
+	}
+
+	// Parse the response to retrieve the relay fee in sat/KB.
+	info := struct {
+		RelayFee float64 `json:"relayfee"`
+	}{}
+	if err := json.Unmarshal(resp, &info); err != nil {
+		return err
+	}
+
+	relayFee, err := btcutil.NewAmount(info.RelayFee)
+	if err != nil {
+		return err
+	}
+
+	// The fee rate is expressed in sat/KB, so we'll manually convert it to
+	// our desired sat/vbyte rate.
+	b.minFeeRate = SatPerVByte(relayFee / 1000)
+
 	return nil
 }
 
@@ -304,12 +366,20 @@ func (b *BitcoindFeeEstimator) fetchEstimatePerVSize(
 	// The value returned is expressed in fees per KB, while we want
 	// fee-per-byte, so we'll divide by 1000 to map to satoshis-per-byte
 	// before returning the estimate.
-	satPerByte := satPerKB / 1000
+	satPerByte := SatPerVByte(satPerKB / 1000)
+
+	// Before proceeding, we'll make sure that this fee rate respects the
+	// minimum relay fee set on the backend node.
+	if satPerByte < b.minFeeRate {
+		walletLog.Debugf("Using backend node's minimum relay fee rate "+
+			"of %v sat/vbyte", b.minFeeRate)
+		satPerByte = b.minFeeRate
+	}
 
 	walletLog.Debugf("Returning %v sat/vbyte for conf target of %v",
 		int64(satPerByte), confTarget)
 
-	return SatPerVByte(satPerByte), nil
+	return satPerByte, nil
 }
 
 // A compile-time assertion to ensure that BitcoindFeeEstimator implements the

lnwallet/interface_test.go

@@ -305,8 +305,14 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("unable to initialize funding reservation: %v", err)
 	}
 	aliceChanReservation.SetNumConfsRequired(numReqConfs)
-	aliceChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
+	err = aliceChanReservation.CommitConstraints(
-		lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10)
+		csvDelay, lnwallet.MaxHTLCNumber/2,
+		lnwire.NewMSatFromSatoshis(fundingAmount), 1, fundingAmount/100,
+		lnwallet.DefaultDustLimit(),
+	)
+	if err != nil {
+		t.Fatalf("unable to verify constraints: %v", err)
+	}
 
 	// 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
@@ -328,8 +334,14 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
 	if err != nil {
 		t.Fatalf("bob unable to init channel reservation: %v", err)
 	}
-	bobChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
+	err = bobChanReservation.CommitConstraints(
-		lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10)
+		csvDelay, lnwallet.MaxHTLCNumber/2,
+		lnwire.NewMSatFromSatoshis(fundingAmount), 1, fundingAmount/100,
+		lnwallet.DefaultDustLimit(),
+	)
+	if err != nil {
+		t.Fatalf("unable to verify constraints: %v", err)
+	}
 	bobChanReservation.SetNumConfsRequired(numReqConfs)
 
 	assertContributionInitPopulated(t, bobChanReservation.OurContribution())
@@ -675,8 +687,14 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 		t.Fatalf("unable to init channel reservation: %v", err)
 	}
 	aliceChanReservation.SetNumConfsRequired(numReqConfs)
-	aliceChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
+	err = aliceChanReservation.CommitConstraints(
-		lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10)
+		csvDelay, lnwallet.MaxHTLCNumber/2,
+		lnwire.NewMSatFromSatoshis(fundingAmt), 1, fundingAmt/100,
+		lnwallet.DefaultDustLimit(),
+	)
+	if err != nil {
+		t.Fatalf("unable to verify constraints: %v", err)
+	}
 
 	// Verify all contribution fields have been set properly.
 	aliceContribution := aliceChanReservation.OurContribution()
@@ -698,8 +716,14 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
 	if err != nil {
 		t.Fatalf("unable to create bob reservation: %v", err)
 	}
-	bobChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2,
+	err = bobChanReservation.CommitConstraints(
-		lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10)
+		csvDelay, lnwallet.MaxHTLCNumber/2,
+		lnwire.NewMSatFromSatoshis(fundingAmt), 1, fundingAmt/100,
+		lnwallet.DefaultDustLimit(),
+	)
+	if err != nil {
+		t.Fatalf("unable to verify constraints: %v", err)
+	}
 	bobChanReservation.SetNumConfsRequired(numReqConfs)
 
 	// We'll ensure that Bob's contribution also gets generated properly.

lnwallet/reservation.go

@@ -284,7 +284,7 @@ func (r *ChannelReservation) SetNumConfsRequired(numConfs uint16) {
 // if the parameters are seemed unsound.
 func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16,
 	maxValueInFlight, minHtlc lnwire.MilliSatoshi,
-	chanReserve btcutil.Amount) error {
+	chanReserve, dustLimit btcutil.Amount) error {
 
 	r.Lock()
 	defer r.Unlock()
@@ -296,6 +296,12 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16,
 		return ErrCsvDelayTooLarge(csvDelay, maxDelay)
 	}
 
+	// The dust limit should always be greater or equal to the channel
+	// reserve. The reservation request should be denied if otherwise.
+	if dustLimit > chanReserve {
+		return ErrChanReserveTooSmall(chanReserve, 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
@@ -331,6 +337,12 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16,
 			minNumHtlc*minHtlc)
 	}
 
+	// Our dust limit should always be less than or equal our proposed
+	// channel reserve.
+	if r.ourContribution.DustLimit > chanReserve {
+		r.ourContribution.DustLimit = chanReserve
+	}
+
 	r.ourContribution.ChannelConfig.CsvDelay = csvDelay
 	r.ourContribution.ChannelConfig.ChanReserve = chanReserve
 	r.ourContribution.ChannelConfig.MaxAcceptedHtlcs = maxHtlcs