From 3c33a8cb6722fc8dbd7cce1e3a8751ab8f09dd5d Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 14 May 2018 13:45:39 -0400 Subject: [PATCH 1/3] lnwallet: enforce backend node's min relay fee floor In this commit, we fix an issue where sometimes transactions wouldn't provide enough of a fee to be relayed by the backend node. This would especially cause issues when sweeping outputs after a contract breach, etc. Now, we'll fetch the minimum relay fee from the backend node and ensure it is set as a lower bound if the estimated fee ends up dipping below it. --- lnwallet/fee_estimator.go | 78 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/lnwallet/fee_estimator.go b/lnwallet/fee_estimator.go index 3bdcdf66..3ff94d9e 100644 --- a/lnwallet/fee_estimator.go +++ b/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 From 2e076ba21e9524b0da76e3586e1b94c97c9f4d5f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 14 May 2018 14:03:58 -0400 Subject: [PATCH 2/3] fundingmanager+lnd: propose remote channel reserve above dust limit --- fundingmanager.go | 19 ++++++++----------- fundingmanager_test.go | 11 +++++++++-- lnd.go | 15 ++++++++++++--- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 49289c01..e3cd3d94 100644 --- a/fundingmanager.go +++ b/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 - // peer's required channel reserve that is to be adhered to at all - // times. - RequiredRemoteChanReserve func(btcutil.Amount) btcutil.Amount + // channel capacity and dust limit, will return an appropriate amount + // for the remote peer's required channel reserve that is to be adhered + // to at all times. + 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 @@ -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 - // delay we require given the total amount of funds within the channel. + // Generate our required constraints for the remote party. remoteCsvDelay := f.cfg.RequiredRemoteDelay(amt) - - // We'll also generate our required constraints for the remote party, - chanReserve := f.cfg.RequiredRemoteChanReserve(amt) + chanReserve := f.cfg.RequiredRemoteChanReserve(amt, msg.DustLimit) maxValue := f.cfg.RequiredRemoteMaxValue(amt) maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(amt) minHtlc := f.cfg.DefaultRoutingPolicy.MinHTLC @@ -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) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index 7c83aec2..d74c2054 100644 --- a/fundingmanager_test.go +++ b/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 { - return chanAmt / 100 + RequiredRemoteChanReserve: func(chanAmt, + 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) diff --git a/lnd.go b/lnd.go index 8aefdaeb..0ca432f6 100644 --- a/lnd.go +++ b/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. - return chanAmt / 100 + // times. If this value ends up dipping below the dust + // 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 From 3a982063a08b4faa62b14de9b3c96cd9fae59e18 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 14 May 2018 14:04:34 -0400 Subject: [PATCH 3/3] fundingmanager+lnwallet: ensure proposed channel reserve is above dust limit --- fundingmanager.go | 6 +++--- lnwallet/errors.go | 9 +++++++++ lnwallet/interface_test.go | 40 ++++++++++++++++++++++++++++++-------- lnwallet/reservation.go | 14 ++++++++++++- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index e3cd3d94..29f12c00 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -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, ) @@ -1146,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) diff --git a/lnwallet/errors.go b/lnwallet/errors.go index 1e113473..403876c3 100644 --- a/lnwallet/errors.go +++ b/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, diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 3bd5f8e3..a549fe86 100644 --- a/lnwallet/interface_test.go +++ b/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, - lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10) + err = aliceChanReservation.CommitConstraints( + 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, - lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10) + err = bobChanReservation.CommitConstraints( + 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, - lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10) + err = aliceChanReservation.CommitConstraints( + 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, - lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10) + err = bobChanReservation.CommitConstraints( + 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. diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index 554b691a..e8a22f47 100644 --- a/lnwallet/reservation.go +++ b/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